Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

SSHLauncher tweaks #2270

Merged
merged 2 commits into from

2 participants

@minrk
Owner
  • use pipes.quote on SSH commands
  • default SSHEngineSetLauncher to ipengine instead of import-based command.

This is actually the intended behavior, and only brings EngineSet inline with SSHEngine and SSHController, which both use the simplified command.

  • Should be backported to 0.13.1
  • Must be not be merged before #2269
@fperez
Owner

Test results for commit 177248f merged into master (fb439f4)
Platform: linux2

  • python2.7: OK
  • python3.2: OK (libraries not available: cython matplotlib oct2py pygments pymongo rpy2 wx wx.aui)

Not available for testing: python2.6

@fperez fperez commented on the diff
IPython/parallel/apps/launcher.py
@@ -584,7 +584,7 @@ def _user_changed(self, name, old, new):
def find_args(self):
return self.ssh_cmd + self.ssh_args + [self.location] + \
- self.program + self.program_args
+ list(map(pipes.quote, self.program + self.program_args))
@fperez Owner
fperez added a note

Why the explicit call to list here? map always returns a list, so I'm not sure why you need this one...

@minrk Owner
minrk added a note

Habits from fighting against Python 3. I don't know if 2to3 always takes care of the list(map), since map returns a generator, and list + map is illegal in py3.

@fperez Owner
fperez added a note

Yeah, let's leave it... Might as well start getting into a py3 mindset, sooner or later we'll be moving over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
IPython/parallel/apps/ipengineapp.py
@@ -226,7 +226,7 @@ def load_connector_file(self):
location = config.EngineFactory.location
proto, ip = d['interface'].split('://')
- ip = disambiguate_ip_address(ip)
+ ip = disambiguate_ip_address(ip, location)
@fperez Owner
fperez added a note

This was basically #2269, no? I'm confused as to why the duplication, since you'd indicated that #2269 should be done first, then I figured you'd base it off that...

@minrk Owner
minrk added a note

This depends on #2269. They are separate because this one should be backported and that one should not. This will not be in the diff once #2269 is merged.

@minrk Owner
minrk added a note

Needed a quick rebase to pull it out of the merge, but it's gone now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
minrk added some commits
@minrk minrk quote args passed through SSH in SSHLaunchers
they need double-quoting for some reason, when passing through SSH.
64fa5db
@minrk minrk default to just `ipengine` for SSHEngineSetLauncher
SSHEngineLauncher and SSHControllerLauncher already had this value set,
but it was missed in SSHEngineSetLauncher, which usually overrides SSHEngineLauncher.
4f820a1
@fperez
Owner

OK, with the rebase and discussion, this is good to go. Thanks!! Merging now.

@fperez fperez merged commit 17840b9 into ipython:master
@minrk minrk referenced this pull request from a commit
@minrk minrk Backport PR #2270: SSHLauncher tweaks
* use pipes.quote on SSH commands
* default SSHEngineSetLauncher to `ipengine` instead of import-based command.

This is actually the intended behavior, and only brings EngineSet inline with SSHEngine and SSHController, which both use the simplified command.

* Should be backported to 0.13.1
* Must be not be merged before #2269
a7c5d8f
@minrk minrk referenced this pull request from a commit
@minrk minrk Backport PR #2270: SSHLauncher tweaks
* use pipes.quote on SSH commands
* default SSHEngineSetLauncher to `ipengine` instead of import-based command.

This is actually the intended behavior, and only brings EngineSet inline with SSHEngine and SSHController, which both use the simplified command.

* Should be backported to 0.13.1
* Must be not be merged before #2269
25ab8ce
@minrk minrk deleted the minrk:sshlaunchers branch
@yarikoptic yarikoptic referenced this pull request from a commit in yarikoptic/ipython
@yarikoptic yarikoptic Merge commit 'rel-0.13-33-gcfc5692' into debian-01X
* commit 'rel-0.13-33-gcfc5692': (33 commits)
  Backport PR #2347: adjust division error message checking to account for Python 3
  Backport PR #2305: RemoteError._render_traceback_ calls self.render_traceback
  Backport PR #2280: fix SSH passwordless check for OpenSSH
  Backport PR #2270: SSHLauncher tweaks
  Backport PR #2261: Fix: longest_substr([]) -> ''
  Backport PR #2250: fix html in notebook example
  Backport PR #2235: remove spurious print statement from setupbase.py
  fixup
  Backport PR #2223: Custom tracebacks
  Backport PR #2214: use KernelApp.exec_lines/files in IPEngineApp
  Backport PR #2212: catch errors in markdown javascript
  Backport PR #2194: clean nan/inf in json_clean
  Backport PR #2177: remove numpy install from travis/tox scripts
  Backport PR #2169: ipdb: pdef, pdoc, pinfo magics all broken
  Backport PR #2186: removed references to h5py dependence in octave magic documentation
  Backport PR #2185: added test for %store, fixed storemagic
  Backport PR #2170: Fix tab completion with IPython.embed_kernel().
  Backport PR #2163: fix 'remote_profie_dir' typo in SSH launchers
  Backport PR #2117: use explicit url in notebook example
  Backport PR #2126: ipcluster broken with any batch (PBS/LSF/SGE)
  ...
fc7c7f5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 9, 2012
  1. @minrk

    quote args passed through SSH in SSHLaunchers

    minrk authored
    they need double-quoting for some reason, when passing through SSH.
  2. @minrk

    default to just `ipengine` for SSHEngineSetLauncher

    minrk authored
    SSHEngineLauncher and SSHControllerLauncher already had this value set,
    but it was missed in SSHEngineSetLauncher, which usually overrides SSHEngineLauncher.
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 1 deletion.
  1. +4 −1 IPython/parallel/apps/launcher.py
View
5 IPython/parallel/apps/launcher.py
@@ -584,7 +584,7 @@ def _user_changed(self, name, old, new):
def find_args(self):
return self.ssh_cmd + self.ssh_args + [self.location] + \
- self.program + self.program_args
+ list(map(pipes.quote, self.program + self.program_args))
@fperez Owner
fperez added a note

Why the explicit call to list here? map always returns a list, so I'm not sure why you need this one...

@minrk Owner
minrk added a note

Habits from fighting against Python 3. I don't know if 2to3 always takes care of the list(map), since map returns a generator, and list + map is illegal in py3.

@fperez Owner
fperez added a note

Yeah, let's leave it... Might as well start getting into a py3 mindset, sooner or later we'll be moving over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
def _send_file(self, local, remote):
"""send a single file"""
@@ -726,6 +726,9 @@ class SSHEngineSetLauncher(LocalEngineSetLauncher):
help="""dict of engines to launch. This is a dict by hostname of ints,
corresponding to the number of engines to start on that host.""")
+ def _engine_cmd_default(self):
+ return ['ipengine']
+
@property
def engine_count(self):
"""determine engine count from `engines` dict"""
Something went wrong with that request. Please try again.