Skip to content

Commit

Permalink
fix bug in slurm_runner for empty commands (#819)
Browse files Browse the repository at this point in the history
- fix bug due to joining of NoneType (no command specified)
- change `dockerfile` variable name to `build_ctx_path` to be consistent with other runners
  • Loading branch information
JayjeetAtGithub committed Apr 27, 2020
1 parent 994c8ec commit 489d489
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
11 changes: 7 additions & 4 deletions cli/popper/runner_slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ def run(self, step):
cid = pu.sanitized_name(step['name'], self._config.wid)
cmd = []

build, img, tag, dockerfile = self._get_build_info(step)
build, img, tag, build_ctx_path = self._get_build_info(step)

cmd.append(f'docker rm -f {cid} || true')

if build:
cmd.append(f'docker build -t {img}:{tag} {dockerfile}')
cmd.append(f'docker build -t {img}:{tag} {build_ctx_path}')
elif not self._config.skip_pull and not step.get('skip_pull', False):
cmd.append(f'docker pull {img}:{tag}')

Expand Down Expand Up @@ -138,15 +138,18 @@ def _create_cmd(self, step, img, cid):
for env_key, env_val in container_args.pop('environment').items():
cmd.append(f'-e {env_key}={env_val}')

command = ' '.join(container_args.pop('command', []))
command = container_args.pop('command')
image = container_args.pop('image')

# anything else is treated as a flag
for k, v in container_args.items():
cmd.append(pu.key_value_to_flag(k, v))

# append the image and the commands
cmd.append(f'{image} {command}')
cmd.append(image)

if command:
cmd.append(' '.join(command))

return ' '.join(cmd)

Expand Down
19 changes: 19 additions & 0 deletions cli/test/test_runner_slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,25 @@ def test_run(self, mock_kill):
docker create --name popper_1_123abc --workdir /workspace --entrypoint cat -v /w:/workspace -v /var/run/docker.sock:/var/run/docker.sock -v /path/in/host:/path/in/container -e FOO=bar --privileged --hostname popper.local --domainname www.example.org popperized/bin:master README.md
docker start --attach popper_1_123abc""")

with WorkflowRunner(config) as r:
wf = YMLWorkflow("""
version: '1'
steps:
- uses: 'popperized/bin/sh@master'
runs: [ls]
""")
wf.parse()
r.run(wf)

with open('/tmp/popper/slurm/popper_1_123abc.sh', 'r') as f:
content = f.read()
self.assertEqual(content,
f"""#!/bin/bash
docker rm -f popper_1_123abc || true
docker build -t popperized/bin:master {os.environ['HOME']}/.cache/popper/123abc/github.com/popperized/bin/sh
docker create --name popper_1_123abc --workdir /workspace --entrypoint ls -v /w:/workspace -v /var/run/docker.sock:/var/run/docker.sock -v /path/in/host:/path/in/container -e FOO=bar --privileged --hostname popper.local --domainname www.example.org popperized/bin:master
docker start --attach popper_1_123abc""")


class TestSlurmSingularityRunner(unittest.TestCase):
def setUp(self):
Expand Down

0 comments on commit 489d489

Please sign in to comment.