-
Notifications
You must be signed in to change notification settings - Fork 184
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
Test mongo postgres mysql tunnels and no hangs anymore :) #219
Conversation
pahaz
commented
Nov 18, 2020
•
edited
Loading
edited
- Changed the daemon flag for all tunnel threads (is not fully backward compatible)
- Add end to end databases tests for Mongo Postgres MySQL
- Add end to end hangs tests
@@ -45,7 +45,7 @@ | |||
#: Timeout (seconds) for tunnel connection (open_channel timeout) | |||
TUNNEL_TIMEOUT = 10.0 | |||
|
|||
_DAEMON = False #: Use daemon threads in connections | |||
_DAEMON = True #: Use daemon threads in connections |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Major changes!
Waiting for 0.3.2 release (#218). It will be at 0.4.0+ |
@@ -97,7 +97,7 @@ | |||
# requirements files see: | |||
# https://packaging.python.org/en/latest/requirements.html | |||
install_requires=[ | |||
'paramiko>=1.15.2', | |||
'paramiko>=2.7.2', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as soon as we're just supporting python >=2.7 that's ok
'ed25519': paramiko.Ed25519Key} | ||
'ecdsa': paramiko.ECDSAKey} | ||
if hasattr(paramiko, 'Ed25519Key'): | ||
# NOQA: new in paramiko>=2.2: http://docs.paramiko.org/en/stable/api/keys.html#module-paramiko.ed25519key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dependency now is paramiko>2.7 so, is this redundant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, We will remove it in the future
): | ||
key_types = (paramiko.RSAKey, paramiko.DSSKey, paramiko.ECDSAKey) | ||
if hasattr(paramiko, 'Ed25519Key'): | ||
# NOQA: new in paramiko>=2.2: http://docs.paramiko.org/en/stable/api/keys.html#module-paramiko.ed25519key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this redundant with paramiko >2.7?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, We will remove it in the future
@@ -1605,6 +1612,14 @@ def __enter__(self): | |||
def __exit__(self, *args): | |||
self.stop(force=True) | |||
|
|||
def __del__(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great!
@pahaz we should disable the requirement for coverage increase. It's OK to have an indicator of how much it's covered but not without that much pressure on % |
@fernandezcuesta yes, it's better to set it as optional ... I want to ship it |