pepkit / looper

A job submitter for Portable Encapsulated Projects
http://looper.databio.org
BSD 2-Clause "Simplified" License
20 stars 7 forks source link

More flexible command execution. #280

Closed nsheff closed 1 year ago

nsheff commented 4 years ago

Came across an issue here: https://github.com/refgenie/refgenomes.databio.org/issues/4

Here's how I solved the problem: https://github.com/pepkit/looper/commit/9eeecfbdc06143d5c9f37fc1894ee22db51b7a18

since running things locally is actually executing these interfaces...

we need a better solution to this. One idea is to make the submission scripts executable and then actually execute them, instead of shell sourcing them. That way the piface command can be anything exectuable, and looper doesn't have to spawn a subshell for this.

nsheff commented 1 year ago

Ok, I got it. I think we should in fact do as I said before:

One idea is to make the submission scripts executable and then actually execute them, instead of shell sourcing them. That way the piface command can be anything exectuable, and looper doesn't have to spawn a subshell for this.

Background:

  1. Looper uses concentric templates to submit jobs.
  2. The actual command run by looper when submitting a job is this: subprocess.check_call(submission_command, shell=True)
  3. The submission_command, used to execute the template, is configurable in the divvy config file; for example, it is sbatch for the default slurm divvy package.
  4. But the above code, with shell=True, is actually not directly running sbatch; instead, it's creating a shell, and then uses that shell to execute sbatch (or whatever) inside that shell.
  5. The problem with this is that the containing shell determines what's allowed in the shell script. This leads to problems like this: https://github.com/refgenie/refgenomes.databio.org/issues/4

Solution:

Can we just remove shell=True from this? or make it customizable?

I've implemented this now; it relies on an update to divvy that will make written submit scripts executable. but I think this is the way to go.