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

Catch TLSParameterErrors from docker-py #2692

Merged
merged 1 commit into from
Jan 19, 2016

Conversation

aanand
Copy link

@aanand aanand commented Jan 18, 2016

No description provided.

@shin-
Copy link

shin- commented Jan 18, 2016

LGTM!

log.error(
'TLS configuration is invalid - make sure your DOCKER_TLS_VERIFY and DOCKER_CERT_PATH are set correctly.\n'
'You might need to run `eval "$(docker-machine env default)"`')
sys.exit(1)
Copy link

Choose a reason for hiding this comment

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

This error is really specific to one section of the code, I don't think we need it in the general catch-all error handling.

Could we move it to docker_client.py and re-raise a UserError with this message?

Arguably a few of these other errors should be moved to more specific areas as well.

Copy link
Author

Choose a reason for hiding this comment

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

Good point - updated.

Some other errors might be moveable - the only ones that need to be here are those which don't happen until we make an API request, I suppose.

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
@dnephin
Copy link

dnephin commented Jan 18, 2016

LGTM

aanand added a commit that referenced this pull request Jan 19, 2016
Catch TLSParameterErrors from docker-py
@aanand aanand merged commit 5b2b4cb into docker:master Jan 19, 2016
@aanand aanand deleted the fix-tls-paramater-error branch January 19, 2016 11:25
@dnephin dnephin added this to the 1.6.0 milestone Jan 19, 2016
@aanand
Copy link
Author

aanand commented Jan 26, 2016

Cherry-picked into #2753

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.

None yet

4 participants