Make SSH_TIMEOUT an overridable argument

[thatmattlove]

Hello,

I'm working to integrate this awesome project into a project of mine, but kept running into an issue when testing connectivity to an SSH proxy server that was unreachable. In my tests, the tunnel connection took a full 120 seconds to time out and raise an exception. When I modified the original constant SSH_TIMEOUT to, say, 10, the session timed out after 10 seconds as was useful for my purposes.

In this PR I set a generous default timeout period of 30 seconds, but this could be adjusted if needed.

Thanks!

[coveralls]

Coverage remained the same at 91.948% when pulling 6a363af4687cdf518105320d2d6e77ac5abec1b6 on checktheroads:master into 397ee78851bdd36d0c11cef319a9f55dd7eb115d on pahaz:master.

[pahaz]

This change is not backward compatible. It looks nice, but what the reason?

[thatmattlove]

Hi @pahaz - sorry for the delay in response. The reason is that the default timeout period is too long to detect an inactive gateway. In my project, if a session is initiated to a device that is unreachable or otherwise inaccessible, the default 120s timeout must expire prior to any further action taking place which is a problem in my case. Can you provide more detail on how this change is not backwards compatible? It seems to me that the codebase relies on calling self.gateway_timeout in the class instance, which is currently card coded. I'd be happy to make other changes needed to make this backwards-compatible, I'm just not sure what would need to change.

Thanks!

[nickodell]

@checktheroads It seems to me that if you made the default None, then that would be backwards compatible with the current behavior. (i.e. if someone has a server which takes 31 seconds to respond, their code will keep working when they upgrade to the new version.)

This does mean that people would only benefit from having a timeout if they explicitly set one. In general, I do think it is a good idea to have timeouts by default, but I think it's also reasonable to insist on backwards compatibility.

[thatmattlove]

@nickodell thank you - wasn't following at first, but your mention of None made me look at the commit and see that I did not set the default to None, which is what I thought I did. Fixed in 71c80871.

@pahaz - will this suffice?

PS: something appears to be wrong with CircleCI, I see the tests are failing on this commit due to ImportError at from coverage.report import Reporter in the test process.

[nickodell]

PS: something appears to be wrong with CircleCI, I see the tests are failing on this commit due to ImportError at from coverage.report import Reporter in the test process.

It does pass tests in Python 3.4, though. 🎉

It looks like the test error is related to this issue: https://github.com/coveralls-clients/coveralls-python/issues/213

sshtunnel depends on tox, which depends on the coverage library. Coveralls also depends on coverage library, so the first time you install dependencies, you install a version which is incompatible with coveralls.

We should try one of the recommendations in that thread:

My best guess would be this is related to the recent coverage==5.0 release. We have an issue to fix this (https://github.com/coveralls-clients/coveralls-python/issues/203) but I've been traveling a few weeks now and haven't had a chance to fix things up.

This coveralls library has a pin to coverage<5, but if you're installing coverage separately, you may be sidestepping this version pin -- could you try doing a pip install coverage<5?

[pahaz]

Please update from master. It looks like a conflict with https://github.com/pahaz/sshtunnel/pull/195/files

[eladavron]

Just my additional 2 cents: I encountered an issue with my project where the gateway was stuck on trying to connect to a downed host for a couple of days and had to be terminated manually, where introducing the exact change this PR is suggesting (and overriding the timeout to something sensible) solved it.

[ignacioparicio]

Same problem in my project. Using sshtunnel==0.4.0, SSH_TIMEOUT does not seem to be configurable