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

ACR audience should be optional with a default value, and not required #22229

Closed
lmazuel opened this issue Dec 17, 2021 · 4 comments · Fixed by #27675
Closed

ACR audience should be optional with a default value, and not required #22229

lmazuel opened this issue Dec 17, 2021 · 4 comments · Fixed by #27675
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Container Registry
Milestone

Comments

@lmazuel
Copy link
Member

lmazuel commented Dec 17, 2021

Today we need to write:

client = ContainerRegistryClient(end_point, DefaultAzureCredential(), audience="https://management.azure.com")

Audience should be optional. Please check the right default in other languages.

@lmazuel lmazuel added bug This issue requires a change to an existing behavior in the product in order to be resolved. Container Registry Client This issue points to a problem in the data-plane of the library. labels Dec 17, 2021
@YalinLi0312
Copy link

Will update until we have a default ACR audience from the service.

@YalinLi0312 YalinLi0312 added this to the Backlog milestone Dec 17, 2021
@YalinLi0312 YalinLi0312 assigned pallavit and unassigned AlexGhiondea Oct 25, 2022
@pallavit
Copy link

I vaguely remember this was a conscious call we had made at the time but I do not recollect the reason :|. I believe some of our SDKs today do have a default especially for national cloud support does it makes sense for us to do the same for ACR as well?
/cc: @annelo-msft , @lmolkova

@lmolkova
Copy link
Member

lmolkova commented Nov 16, 2022

ACR for Java defaults to public cloud. But it's required on .NET (it was introduced here Azure/azure-sdk-for-net#23141, it seems there was a need to postpone choosing default value).

BTW I never saw another SDK that requires an audience, so let's try to make it optional unless there is a strong reason not to. And of course, make it consistent.

@lmolkova
Copy link
Member

Found the guideline: https://azure.github.io/azure-sdk/policies_support.html#azure-clouds

By default, the Azure libraries are configured to connect to the Global Azure Cloud.

azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this issue Feb 8, 2023
Update Cadl sample project (Azure#22229)

* Update Cadl sample project

- Move sample project under specification folder
- remove project.json file from individual project
- Add global project.json file under specifications folder
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Container Registry
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants