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

WIP: Update to use AzureCliCredential instead of key #81

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Eilon
Copy link
Member

@Eilon Eilon commented Oct 1, 2024

Note: This can't be tested right now due to an infrastructure issue (that we talked about in Teams)

Just putting it here in case that issue gets resolved, and then we have code/docs to work from. Happy to test it out when possible.

Note: This can't be tested right now due to an infrastructure issue
@Eilon Eilon requested a review from jeffhandley October 1, 2024 20:46
BlobContainerClient container;
try
{
container = new BlobContainerClient(azureStorageConnectionString, blobContainerName: "areamodels");
container = new BlobContainerClient(new Uri("https://dotnetissuelabelerdata.blob.core.windows.net/areamodels"), new AzureCliCredential());
Copy link
Member

Choose a reason for hiding this comment

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

We can use new DefaultAzureCredential() here instead. This way it will try to resolve Azure credentials from a number of sources (including CLI) which might work better for people who are already authenticated into Azure in Visual Studio or VS Code.

BlobContainerClient container;
try
{
container = new BlobContainerClient(azureStorageConnectionString, blobContainerName: "areamodels");
container = new BlobContainerClient(new Uri("https://dotnetissuelabelerdata.blob.core.windows.net/areamodels"), new AzureCliCredential());
Copy link
Member

Choose a reason for hiding this comment

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

It's probably not a good idea to hardcode the storage account name. Instead, it should be configurable.
We'll need to make the same (or similar) change in other places in the solution.

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