Skip to content
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

azure: Fix ignored options when creating shared credential Azure client #34

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

phillebaba
Copy link
Contributor

@phillebaba phillebaba commented Nov 8, 2022

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Set client options when creating a Azure client with shared key. Before this the option was only set to clients using MSI authentication.

Verification

Should not require verification as the same option is passed to the MSI client.

@phillebaba phillebaba force-pushed the fix/azure-shared-key-opt branch 2 times, most recently from 7cce123 to 5623bbc Compare November 8, 2022 18:12
Signed-off-by: Philip Laine <philip.laine@gmail.com>
@phillebaba phillebaba force-pushed the fix/azure-shared-key-opt branch from 5623bbc to 5db0865 Compare November 8, 2022 18:13
@phillebaba
Copy link
Contributor Author

Fixes thanos-io/thanos/issues/5874

@phillebaba
Copy link
Contributor Author

@ahurtaud could you build a fork with this branch of objstore and verify that it works for you?

@ahurtaud
Copy link

ahurtaud commented Nov 9, 2022

Sure I should be able to do this tomorrow. Otherwise on monday ;)

Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ahurtaud
Copy link

@phillebaba I build thanos main + your objstore fix/azure-shared-key-opt branch and it works (tested thanos store only but I guess it'll work for all)
ready to merge.
thank you for the very quick fix!

@GiedriusS GiedriusS merged commit 9779816 into thanos-io:main Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants