Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ipcluster broken with any batch launcher (PBS/LSF/SGE) #2126

Merged
merged 1 commit into from Jul 15, 2012
Merged

ipcluster broken with any batch launcher (PBS/LSF/SGE) #2126

merged 1 commit into from Jul 15, 2012

Conversation

guyhf
Copy link
Contributor

@guyhf guyhf commented Jul 13, 2012

I have setup ipcluster_config.py to start with LSF:

c.IPClusterStart.controller_launcher_class = 'LSF'
c.IPClusterStart.engine_launcher_class = 'LSF'

But the ipcluster command fails to start the engines:

ipcluster start --profile=lsf -n 10

The problem is fixed if I add quotes to the launch command string cmd in launcher.py.

diff --git a/IPython/parallel/apps/launcher.py b/IPython/parallel/apps/launcher.py
index e752d2a..6035303 100644
--- a/IPython/parallel/apps/launcher.py
+++ b/IPython/parallel/apps/launcher.py
@@ -73,7 +73,7 @@ WINDOWS = os.name == 'nt'
 # Paths to the kernel apps
 #-----------------------------------------------------------------------------

-cmd = "from IPython.parallel.apps.%s import launch_new_instance; launch_new_instance()"
+cmd = "\"from IPython.parallel.apps.%s import launch_new_instance; launch_new_instance()\""

 ipcluster_cmd_argv = [sys.executable, "-c", cmd % "ipclusterapp"]

@bfroehle
Copy link
Contributor

Unfortunately this fix will break the other launchers which call processes directly (instead of writing a batch script).

Instead can you look at changing the LSF...Launcher classes in that same file?

I think something like replacing ' '.join(ipcluster_cmd_argv) with subprocess.list2cmdline(ipcluster_cmd_argv) might work.

@bfroehle
Copy link
Contributor

You may want to look into pipes.quote as in ' '.join(map(pipes.quote, ipcluster_cmd_argv)) too. There's some discussion that subprocess.list2cmdline doesn't handle various special characters like | correctly.

@guyhf
Copy link
Contributor Author

guyhf commented Jul 12, 2012

That makes sense. I moved the change into the Launcher classes as you suggested. The following fix works for LSF.

guyhf@0e59251

Happy to send a pull request.

@bfroehle
Copy link
Contributor

Yes, please make a pull request. If this fix seems appropriate elsewhere
in the file we should apply it there as well.

Lastly, two small styles changes:

  1. Move the import pipes up a few lines to maintain alphabetical order.
  2. Put a space after the comma in map(pipes.quote, ...)

Thanks!

@minrk
Copy link
Member

minrk commented Jul 14, 2012

This looks good to me now, thanks @bfroehle for the review! Anything else before merge?

Side question: does anyone know how this Issue magically turned itself into a pull request?

@bfroehle
Copy link
Contributor

The one way I know how to turn issues into pull requests is with defunkt/hub.

@bfroehle
Copy link
Contributor

This looks good to me as well. Thanks @guyhf.

@minrk
Copy link
Member

minrk commented Jul 15, 2012

Thanks, I'll go ahead and merge, then.

I've been using hub for ages, but I didn't know you could do that. Neat!

minrk added a commit that referenced this pull request Jul 15, 2012
@minrk
Merge pull request #2126 from guyhf/ipclusterbatchfix
3c4aa39 
use pipes.quote when writing ipfoo_cmd to batch files

fixes ipcluster with any batch system when there are spaces in these commands.
@minrk minrk merged commit 3c4aa39 into ipython:master Jul 15, 2012
minrk added a commit that referenced this pull request Jul 21, 2012
@minrk
Backport PR #2126: ipcluster broken with any batch (PBS/LSF/SGE)
061632b 
I have setup ipcluster_config.py to start with LSF:
```
c.IPClusterStart.controller_launcher_class = 'LSF'
c.IPClusterStart.engine_launcher_class = 'LSF'
```

But the ipcluster command fails to start the engines:

```
ipcluster start --profile=lsf -n 10
```

The problem is fixed if I add quotes to the launch command string ```cmd``` in ```launcher.py```.

```
diff --git a/IPython/parallel/apps/launcher.py b/IPython/parallel/apps/launcher.py
index e752d2a..6035303 100644
--- a/IPython/parallel/apps/launcher.py
+++ b/IPython/parallel/apps/launcher.py
@@ -73,7 +73,7 @@ WINDOWS = os.name == 'nt'
 # Paths to the kernel apps
 #-----------------------------------------------------------------------------

-cmd = "from IPython.parallel.apps.%s import launch_new_instance; launch_new_instance()"
+cmd = "\"from IPython.parallel.apps.%s import launch_new_instance; launch_new_instance()\""

 ipcluster_cmd_argv = [sys.executable, "-c", cmd % "ipclusterapp"]

 ```
@bfroehle bfroehle mentioned this pull request Oct 1, 2012
Closed
yarikoptic added a commit to yarikoptic/ipython that referenced this pull request May 2, 2014
@yarikoptic
Merge commit 'rel-0.13-33-gcfc5692' into debian-01X
fc7c7f5 
* commit 'rel-0.13-33-gcfc5692': (33 commits)
  Backport PR ipython#2347: adjust division error message checking to account for Python 3
  Backport PR ipython#2305: RemoteError._render_traceback_ calls self.render_traceback
  Backport PR ipython#2280: fix SSH passwordless check for OpenSSH
  Backport PR ipython#2270: SSHLauncher tweaks
  Backport PR ipython#2261: Fix: longest_substr([]) -> ''
  Backport PR ipython#2250: fix html in notebook example
  Backport PR ipython#2235: remove spurious print statement from setupbase.py
  fixup
  Backport PR ipython#2223: Custom tracebacks
  Backport PR ipython#2214: use KernelApp.exec_lines/files in IPEngineApp
  Backport PR ipython#2212: catch errors in markdown javascript
  Backport PR ipython#2194: clean nan/inf in json_clean
  Backport PR ipython#2177: remove numpy install from travis/tox scripts
  Backport PR ipython#2169: ipdb: pdef, pdoc, pinfo magics all broken
  Backport PR ipython#2186: removed references to h5py dependence in octave magic documentation
  Backport PR ipython#2185: added test for %store, fixed storemagic
  Backport PR ipython#2170: Fix tab completion with IPython.embed_kernel().
  Backport PR ipython#2163: fix 'remote_profie_dir' typo in SSH launchers
  Backport PR ipython#2117: use explicit url in notebook example
  Backport PR ipython#2126: ipcluster broken with any batch (PBS/LSF/SGE)
  ...
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@minrk
Merge pull request ipython#2126 from guyhf/ipclusterbatchfix
39dca5e 
use pipes.quote when writing ipfoo_cmd to batch files

fixes ipcluster with any batch system when there are spaces in these commands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug important parallel
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@guyhf @bfroehle @minrk