-
Notifications
You must be signed in to change notification settings - Fork 817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: migrate output azure_blob_storage to azure-sdk-for-go/storage/azblob #1901
feat: migrate output azure_blob_storage to azure-sdk-for-go/storage/azblob #1901
Conversation
…zblob Signed-off-by: eduardodbr <eduardodbr@hotmail.com>
Signed-off-by: eduardodbr <eduardodbr@hotmail.com>
Signed-off-by: eduardodbr <eduardodbr@hotmail.com>
c := a.client.GetContainerReference(containerStr) | ||
b := c.GetBlobReference(pathStr) | ||
if err = a.uploadBlob(b, blobTypeStr, p.AsBytes()); err != nil { | ||
ctx := p.GetContext() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to familiarise myself with how this context is used, but shouldn’t we use here the ctx
parameter of the method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. I think I got used to retrieve the context from the message on custom processors and messed up here. Thanks!
Hey @eduardodbr, for some reason GitHub won't let me create PRs against your fork... Dunno why :( I took some time to port my changes on top of yours on this branch and I created this PR on my fork where you can leave comments if there's anything that doesn't look right or you'd rather I not change. Once you're happy with my changes, you should be able to just cherry-pick my commit(s). Note that I first rebased your branch on top of the latest BTW, you'll want to use the git commit |
Also fixed a few issues: - Documented when `storage_connection_string` doesn't have `AccountName` in it and added linting rules for it - Ensure that the `blob_storage_append` integration test creates new containers Signed-off-by: Mihai Todor <mihaitodor@optum.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @eduardodbr and @mihaitodor!
Thanks for merging my small changes @eduardodbr! ❤️ |
The output azure_blob_storage uses a deprecated SDK as you can see here. This PR migrates the output to the most recent SDK. It also added the option to authenticate via managed identity when no other auth method is provided (conn string, access key, or SAS token).