Improve error message in case of paramiko failure #2252
Conversation
|
Please sign your commits following these rules: $ git clone -b "fix/improve-ssh-error" [email protected]:ssbarnea/docker-py.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
Fix issues where essential error about paramiko dealing code was lost due to ageneric exception that assumed paramiko library was missing. To keep backwards compatibility we keep the custom exception but we also include information from original exception that caused it. It was wrong to believe that only missing paramiko can cause it, even code inside docker-py can thow exceptions, and NamedException is very broad exception class. Signed-off-by: Sorin Sbarnea <[email protected]>
ssbarnea
force-pushed
the
fix/improve-ssh-error
branch
from
7350b10 to
f919610
Compare
| ) | ||
| except NameError: | ||
| except NameError as e: | ||
| # do not hide orriginal exception as it may contain important info |
There was a problem hiding this comment.
typo: orriginal -> original
| # do not hide orriginal exception as it may contain important info | ||
| raise DockerException( | ||
| 'Install paramiko package to enable ssh:// support' | ||
| "Paramiko failure while trying ssh:// protocol:\n%s" % e |
There was a problem hiding this comment.
Not sure if changing the message is necessary here.
|
@ssbarnea Do you have an example of a My problem with this PR is that I would still like to give clear information to the user in the case they haven't installed the optional |
|
@shin- Yes I encountered that but I do not remember it now. I would rather throw the original error back instead of creating a new one that is crippled of important information. That code is wrong as it makes some dangereus assumptions. |
|
This PR is not passing due to an already solved problem. |
5 participants
Fix issues where essential error about paramiko dealing code was
lost due to ageneric exception that assumed paramiko library was
missing.
To keep backwards compatibility we keep the custom exception but we
also include information from original exception that caused it.
It was wrong to believe that only missing paramiko can cause it, even
code inside docker-py can thow exceptions, and NamedException is very
broad exception class.