Skip to content

Conversation

Melraidin
Copy link
Contributor

This tries to load Docker authentication info from
~/.docker/config.json before falling back to its legacy location and
format at ~/.dockercfg.

Resolves #648

@Melraidin
Copy link
Contributor Author

@mpetazzoni Refactored a bit of that to avoid some duplication. Also allowed the provided config_path to override the default both for the old and new config locations.

@Melraidin
Copy link
Contributor Author

On a side note it'd be nice if we could give some feedback to the caller on whether any authentication was sent. Without this docker-compose is left to simply say that a repo wasn't found with no indication that it could be a simple authentication issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be scoped to the specific type(s) of exception we're expecting?

@Melraidin
Copy link
Contributor Author

@aanand I was aiming to follow the style later in the code for the legacy location where we're ignoring all exceptions but your point's totally valid. Will update.

@shin- shin- added this to the 1.3.0 milestone Jun 25, 2015
@aanand
Copy link
Contributor

aanand commented Jun 25, 2015

Just saw that line, yes - we should fix that too (in a separate PR).

@mpetazzoni
Copy link
Contributor

👍 LGTM. Might be worth squashing those together.

This tries to load Docker authentication info from
~/.docker/config.json before falling back to its legacy location and
format at ~/.dockercfg.

Resolves docker#648
@shin-
Copy link
Contributor

shin- commented Jun 30, 2015

LGTM. Thanks!

shin- added a commit that referenced this pull request Jun 30, 2015
Prefer new Docker config location and format.
@shin- shin- merged commit d300f5f into docker:master Jun 30, 2015
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.

4 participants