From 747b87d3ec82b787c26c7ad95975e787ab7e1a72 Mon Sep 17 00:00:00 2001 From: John Marshall Date: Wed, 25 Sep 2024 10:41:20 +1200 Subject: [PATCH 1/2] [batch] Ignore empty command strings when constructing shell scripts Using job.command('') (or more likely something more complex that sometimes evaluates to an empty string) led to shell snippets containing `... { } ...`, which is a shell syntax error. Prevent this by not adding such strings to self._command and warning. --- hail/python/hailtop/batch/job.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hail/python/hailtop/batch/job.py b/hail/python/hailtop/batch/job.py index 97eb45be0d4..facf34f1e2c 100644 --- a/hail/python/hailtop/batch/job.py +++ b/hail/python/hailtop/batch/job.py @@ -859,6 +859,11 @@ def command(self, command: str) -> 'BashJob': Same job object with command appended. """ + if command.strip() == '': + job_name = f'job {self.name!r}' if self.name else 'unnamed job' + warnings.warn(f'Ignoring empty command specified for {job_name}.') + return self + command = self._interpolate_command(command) self._command.append(command) return self From 600826710afb8dfef5fcbf19f440a43f263ec0ee Mon Sep 17 00:00:00 2001 From: Edmund Higham Date: Tue, 8 Oct 2024 18:40:54 -0400 Subject: [PATCH 2/2] add parameterised test for commands comprised of random whitespace --- .../test/hailtop/batch/test_batch_local_backend.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hail/python/test/hailtop/batch/test_batch_local_backend.py b/hail/python/test/hailtop/batch/test_batch_local_backend.py index dc059237d4c..15e256a6dc8 100644 --- a/hail/python/test/hailtop/batch/test_batch_local_backend.py +++ b/hail/python/test/hailtop/batch/test_batch_local_backend.py @@ -1,6 +1,8 @@ import os import subprocess as sp import tempfile +from random import choice, randint +from string import whitespace from typing import AsyncIterator import pytest @@ -179,6 +181,18 @@ def test_single_job_bad_command(batch): b.run() +@pytest.fixture +def randwhitespace(): + return ''.join(choice(whitespace) for _ in range(randint(0, 9))) + + +@pytest.mark.parametrize('randwhitespace', range(20), indirect=True) +def test_single_job_empty_command(batch, randwhitespace): + j = batch.new_job() + with pytest.warns(match='Ignoring empty command'): + j.command(randwhitespace) + + def test_declare_resource_group(batch): with tempfile.NamedTemporaryFile('w') as output_file: msg = 'hello world'