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] Add a new argument '--expose-token' for 'az acr login' #12077

Merged
merged 3 commits into from
Feb 8, 2020
Merged

[ACR] Add a new argument '--expose-token' for 'az acr login' #12077

merged 3 commits into from
Feb 8, 2020

Conversation

Wwwsylvia
Copy link
Contributor

@Wwwsylvia Wwwsylvia commented Feb 6, 2020

  1. Add a new argument '--expose-token' for 'az acr login' to expose access token.
  2. Update help manual accordingly.

Close #9442

Behavior:
[specify --expose-token]
image

[docker command error]
image

[help manual]
image

History Notes:
(Fill in the following template if multiple notes are needed, otherwise PR title will be used for history note.)

[Component Name 1] (BREAKING CHANGE:) (az command:) make some customer-facing change.
[Component Name 2] (BREAKING CHANGE:) (az command:) make some customer-facing change.


This checklist is used to make sure that common guidelines for a pull request are followed.

@yonzhan yonzhan added this to the S165 milestone Feb 7, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Feb 7, 2020

add to S165.

tenant_suffix=tenant_suffix,
username=username,
password=password)
logger.warning("You can perform manual login with the ouput access token, for example: 'docker login loginServer -u GUID -p accessToken'")
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the GUID here should be 00...1 always. If yes, let us just use the fixed vaule.

}
return token_info

tips = "You may want to use 'az acr login -n MyRegistry --expose-token' to get an access token, which does not require Docker to be installed."
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the real registry name instead of MyRegistry so that users can just copy & paste

from azure.cli.core.util import in_cloud_console
if in_cloud_console():
raise CLIError('This command requires running the docker daemon, which is not supported in Azure Cloud Shell.')
raise CLIError('This command requires running the docker daemon, which is not supported in Azure Cloud Shell.' + tips)
Copy link
Contributor

Choose a reason for hiding this comment

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

You might need a white space string before the Tips

logger.warning("You can perform manual login with the ouput access token, for example: 'docker login loginServer -u GUID -p accessToken'")
token_info = {
"loginServer" : login_server,
"GUID" : username,
Copy link
Contributor

Choose a reason for hiding this comment

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

Again username could be None and if yes, the value will be 000...1.

…gistry' with user's real registry name in tips

2.Revise wording
3.Fix trailing whitespaces
@yugangw-msft yugangw-msft merged commit 0a6bcd2 into Azure:dev Feb 8, 2020
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.

'az acr login' requires Docker to be installed
4 participants