Add SSH tunneling to engines #685

Merged
merged 6 commits into from Aug 16, 2011

Conversation

Projects
None yet
2 participants
@minrk
Member

minrk commented Aug 7, 2011

This copies the basic ssh tunneling from the Client to the Engine. The same semantics apply. In the controller, the ssh server is configured separately from the client ssh server, to prevent unnecessary tunneling from engines to the controller.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Aug 16, 2011

Member

This is super useful, but I think it would be really good to have even a short paragraph illustrating this in the docs. Not all users are familiar with the details of ssh tunneling, so I'm sure a short illustrative example will do lots of good here.

Otherwise, the code looks solid (and the current test suite still passes) but I'm a little concerned that there's no test coverage at all, despite a fair amount of new functionality. I know that testing multiprocess things like this is super tricky, but even some light tests that at least do api validation will help us catch silly mistakes in the future. Obviously if you can think of some robust tests for it, that would be even better.

Beyond some docs and testing, it looks otherwise great for merging. Thanks for the excellent work, this will be very useful!

Member

fperez commented Aug 16, 2011

This is super useful, but I think it would be really good to have even a short paragraph illustrating this in the docs. Not all users are familiar with the details of ssh tunneling, so I'm sure a short illustrative example will do lots of good here.

Otherwise, the code looks solid (and the current test suite still passes) but I'm a little concerned that there's no test coverage at all, despite a fair amount of new functionality. I know that testing multiprocess things like this is super tricky, but even some light tests that at least do api validation will help us catch silly mistakes in the future. Obviously if you can think of some robust tests for it, that would be even better.

Beyond some docs and testing, it looks otherwise great for merging. Thanks for the excellent work, this will be very useful!

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Aug 16, 2011

Member

We simply can't test ssh tunneling except by hand. We can't depend on ssh being installed, or passwordless keys, or permissions, etc. Testing ssh in any meaningful way simply requires the use of multiple machines.

I'll toss up an example in the docs, though.

Member

minrk commented Aug 16, 2011

We simply can't test ssh tunneling except by hand. We can't depend on ssh being installed, or passwordless keys, or permissions, etc. Testing ssh in any meaningful way simply requires the use of multiple machines.

I'll toss up an example in the docs, though.

minrk added some commits Aug 7, 2011

split open_tunnel part of tunnel_connection into separate method
This allows connection forwarding without establishing the final connection

(needed if the final connection is delayed, e.g. heartbeats)
add ssh tunneling to Engine
'enginessh' alias added to ipcontroller to new IPControllerApp.engine_ssh_server

ssh/keyfile added to ipengine/EngineFactory
@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Aug 16, 2011

Member

simple engine ssh example added to parallel docs

Member

minrk commented Aug 16, 2011

simple engine ssh example added to parallel docs

fperez added a commit that referenced this pull request Aug 16, 2011

Merge pull request #685 from minrk/enginessh
Add SSH tunneling to engines

@fperez fperez merged commit 52dffc0 into ipython:master Aug 16, 2011

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #685 from minrk/enginessh
Add SSH tunneling to engines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment