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.

Sign up for GitHub

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

Jump to bottom

feat(): add ManagedIdentity in Azure Blob Storage #4858

Merged
merged 10 commits into from
Dec 15, 2021
Merged

feat(): add ManagedIdentity in Azure Blob Storage #4858

merged 10 commits into from
Dec 15, 2021

Conversation

sandy2008
Copy link
Contributor

  • Support Using ManagedIdentity in Azure Blob Storage

@CLAassistant
Copy link

CLAassistant commented Dec 2, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@hanayashiki
Copy link

wow, this looks super useful

DylanGuedes
DylanGuedes reviewed Dec 2, 2021
View reviewed changes
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

a few nits

pkg/storage/chunk/azure/blob_storage_client.go Outdated Show resolved Hide resolved
pkg/storage/chunk/azure/blob_storage_client.go Outdated Show resolved Hide resolved
pkg/storage/chunk/azure/blob_storage_client.go Show resolved Hide resolved
pkg/storage/chunk/azure/blob_storage_client.go Outdated Show resolved Hide resolved
pkg/storage/chunk/azure/blob_storage_client.go Outdated Show resolved Hide resolved
pkg/storage/chunk/azure/blob_storage_client.go Outdated Show resolved Hide resolved
pkg/storage/chunk/azure/blob_storage_client.go Outdated Show resolved Hide resolved
@gai6948
Copy link

gai6948 commented Dec 7, 2021

looking forward to this feature, want to use that on our AKS clusters

@cyriltovena
Copy link
Contributor

Can you run make check-mod and push the result ? Do you nee help finishing this ?

@sandy2008
Copy link
Contributor Author

Can you run make check-mod and push the result ? Do you nee help finishing this ?

Sorry I was little busy recently, I am wrapping it up!
Thank you so much for your kind comments!

@pull-request-size pull-request-size bot added size/M and removed size/L labels Dec 9, 2021
@sandy2008
Copy link
Contributor Author

a few nits

Thx, I fixed all of them.

@sandy2008 sandy2008 requested review from DylanGuedes and removed request for a team December 9, 2021 17:06
@sandy2008
Copy link
Contributor Author

@DylanGuedes @cyriltovena Please kindly review it again when you got time.

@hanayashiki
Copy link

can‘t wait for this to be merged 🙏

DylanGuedes
DylanGuedes approved these changes Dec 14, 2021
View reviewed changes
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

just a nit but LGTM! (I'm not a maintainer, though)

pkg/storage/chunk/azure/blob_storage_client.go Outdated
return nil, err
}

// defing the Azure Pipeline Options
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

cyriltovena
cyriltovena approved these changes Dec 15, 2021
View reviewed changes
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit fc2bc69 into grafana:main Dec 15, 2021
@sandy2008 sandy2008 mentioned this pull request Jan 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyriltovena cyriltovena cyriltovena approved these changes

@DylanGuedes DylanGuedes DylanGuedes approved these changes

@KMiller-Grafana KMiller-Grafana Awaiting requested review from KMiller-Grafana

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sandy2008 @CLAassistant @hanayashiki @gai6948 @cyriltovena @DylanGuedes