feat: enable Azure blob storage #176
Conversation
| @@ -272,6 +272,13 @@ Template containing common environment variables that are used by several servic | |||
| {{- if .Values.config.blobStorage.enabled }} | |||
| - name: FF_S3_STORAGE_ENABLED | |||
| value: {{ .Values.config.blobStorage.enabled | quote }} | |||
| - name: FF_BLOB_STORAGE_ENABLED | |||
There was a problem hiding this comment.
Will the old values still work?
There was a problem hiding this comment.
old values are used for now, yeah. will start using the new ones after this is released
There was a problem hiding this comment.
sorry i mean like for people not using helm, will the old variables still be read
There was a problem hiding this comment.
ah right, probably worth respecting the old values until 0.9?
There was a problem hiding this comment.
yea though i guess that can just be a backend thing to read both
charts/langsmith/values.yaml
Outdated
| azureStorageAccountKey: "" | ||
| azureStorageContainerName: "" | ||
| azureStorageConnectionString: "" | ||
| azureStorageSASToken: "" |
There was a problem hiding this comment.
Do we need to support all these auth modes? E.g wouldn't really ever expect someone to use SAS or connection string (that's like presignrd url right?)
There was a problem hiding this comment.
I think connection string is useful for the flexibility - it isn't specifically presigned URL, could use account name and key in the string. would prefer to keep that one.
probably fine to remove SAS token and add support only if needed
There was a problem hiding this comment.
if we do wanna support connection string via secret could we just pass these in via connectionString always? Seems like that parameter should also allow you to specify accountKey or SAStoken?
There was a problem hiding this comment.
oops sorry meant accountKey rather than connectionString, think connectionString we should keep for sure since seems like its kind of like apiUrl?
There was a problem hiding this comment.
kind of nice to construct the URL for them with accountKey and secret imo, even if that's just for teams testing out on Azure
There was a problem hiding this comment.
ah gotcha, what happens if both are specified today? i think fine maybe just add a note that x overrides this if manually specified. i just generally have a bias towards connectionString when possible since it allows you to add url params and stuff (e.g when specifying db uris)
There was a problem hiding this comment.
account & key has higher precedence. added a comment here & updated the docs to call this out
No description provided.