Skip to content

Conversation

shin-
Copy link
Contributor

@shin- shin- commented Sep 3, 2016

No description provided.

@shin- shin- added this to the 1.10.0 milestone Sep 3, 2016
@shin-
Copy link
Contributor Author

shin- commented Sep 3, 2016

@shin- shin- force-pushed the credstore-support branch from 21c0404 to 240cf33 Compare September 3, 2016 03:03
@shin-
Copy link
Contributor Author

shin- commented Sep 3, 2016

Fixes #1023

return res
except dockerpycreds.StoreError as e:
log.error('Credentials store error: {0}'.format(repr(e)))

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should raise an exception? The user explicitly asked to use the credential store, right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I need to differentiate a runtime error from a missing entry in dockerpycreds, then we can raise when there's an issue, and proceed with an unauthenticated push/pull if we just don't have credentials.

@dnephin
Copy link
Contributor

dnephin commented Sep 6, 2016

LGTM, just one comment.

I'm not all that familiar with these credential stores, but the code looks good to me.

Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
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.

3 participants