Skip to content

Conversation

@listonb
Copy link

@listonb listonb commented Jun 15, 2017

  • Execute a docker login for a given container registry

@msftclas
Copy link

@listonb,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@codecov-io
Copy link

codecov-io commented Jun 15, 2017

Codecov Report

Merging #3734 into master will decrease coverage by 0.02%.
The diff coverage is 23.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3734      +/-   ##
==========================================
- Coverage   72.09%   72.07%   -0.03%     
==========================================
  Files         422      422              
  Lines       26259    26272      +13     
  Branches     3995     3996       +1     
==========================================
+ Hits        18932    18935       +3     
- Misses       6099     6106       +7     
- Partials     1228     1231       +3
Impacted Files Coverage Δ
...-cli-acr/azure/cli/command_modules/acr/commands.py 100% <100%> (ø) ⬆️
...li-acr/azure/cli/command_modules/acr/credential.py 51.61% <16.66%> (-22.08%) ⬇️
...nent/azure/cli/command_modules/component/custom.py 16.23% <0%> (ø) ⬆️
src/azure-cli-core/azure/cli/core/util.py 70.06% <0%> (ø) ⬆️
...dback/azure/cli/command_modules/feedback/custom.py 31.25% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 299683b...47b8f70. Read the comment docs.

* Execute a docker login output for a given container registry
@listonb listonb changed the title Add Docker Login Command Retrieval for ACR Add Docker Login Command for ACR Jun 15, 2017
@johanste johanste requested a review from djyou June 15, 2017 20:43
@djyou
Copy link
Member

djyou commented Jun 15, 2017

@listonb Thank you for the PR. We planned a PR to be out in the next a few days with our latest support for managed registries, webhooks, AAD-based docker login with az acr login (so that regardless whether admin user is enabled we will log you in) and more repository operations (including delete). For the docker login command specifically, you can take a look at our work in progress here https://github.com/AppPlatPerf/azure-cli/blob/master/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py#L213 Your PR is a subset of what we are preparing except that we definitely want to have an az acr login command rather than az acr credential login. I will cc you once our PR is out. /cc @sajayantony @DavidObando

@listonb
Copy link
Author

listonb commented Jun 15, 2017

@djyou Great. My only addition would be to verify docker is installed which the PR you referred to currently does not. I will close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants