From bc9c6a73b02305959d58f32d9e36f64db80fc038 Mon Sep 17 00:00:00 2001 From: Doug Latornell Date: Sat, 23 Nov 2024 12:10:38 -0800 Subject: [PATCH] Change job submission command for 'salish' to 'bash' Updated the job submission command for the 'salish' system from 'qsub' to 'bash'. Added logic to handle job submissions specifically for 'bash' and adjusted unit tests accordingly to ensure compatibility and functionality. --- salishsea_cmd/run.py | 19 ++++-- tests/test_run.py | 137 +++++++++++++++++++++++++++++-------------- 2 files changed, 109 insertions(+), 47 deletions(-) diff --git a/salishsea_cmd/run.py b/salishsea_cmd/run.py index cda81ee..493171b 100644 --- a/salishsea_cmd/run.py +++ b/salishsea_cmd/run.py @@ -275,7 +275,7 @@ def run( "graham": "sbatch", "omega": "qsub -q mpi", # optimum.eoas.ubc.ca login node "orcinus": "qsub", - "salish": "qsub", + "salish": "bash", "seawolf1": "qsub", # orcinus.westgrid.ca login node "seawolf2": "qsub", # orcinus.westgrid.ca login node "seawolf3": "qsub", # orcinus.westgrid.ca login node @@ -543,12 +543,23 @@ def _build_tmp_run_dir( def _submit_job(batch_file, queue_job_cmd, waitjob): if waitjob != "0": - depend_opt = ( - "-W depend=afterok" if queue_job_cmd.startswith("qsub") else "-d afterok" - ) + match queue_job_cmd.split(" ")[0]: + case "qsub": + depend_opt = "-W depend=afterok" + case "sbatch": + depend_opt = "-d afterok" + case _: + log.error( + f"dependent jobs are not available for systems that launch jobs with " + f"{queue_job_cmd}" + ) + raise SystemExit(2) cmd = f"{queue_job_cmd} {depend_opt}:{waitjob} {batch_file}" else: cmd = f"{queue_job_cmd} {batch_file}" + if queue_job_cmd == "bash": + subprocess.Popen(shlex.split(cmd), start_new_session=True) + return f"{cmd} started" submit_job_msg = subprocess.run( shlex.split(cmd), check=True, universal_newlines=True, stdout=subprocess.PIPE ).stdout diff --git a/tests/test_run.py b/tests/test_run.py index f889432..9bbcb7f 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -18,6 +18,7 @@ """SalishSeaCmd run sub-command plug-in unit tests """ +import logging import os import shlex import subprocess @@ -143,8 +144,8 @@ class TestRun: (True, 4, "delta", "qsub -q mpi", "43.admin.default.domain"), (False, 0, "graham", "sbatch", "Submitted batch job 43"), (True, 4, "graham", "sbatch", "Submitted batch job 43"), - (False, 0, "salish", "qsub", "43.master"), - (True, 4, "salish", "qsub", "43.master"), + (False, 0, "salish", "bash", "bash run_dir/SalishSeaNEMO.sh started"), + (True, 4, "salish", "bash", "bash run_dir/SalishSeaNEMO.sh started"), (False, 0, "sigma", "qsub -q mpi", "43.admin.default.domain"), (True, 4, "sigma", "qsub -q mpi", "43.admin.default.domain"), (False, 0, "sockeye", "sbatch", "Submitted batch job 43"), @@ -176,6 +177,7 @@ def test_run_submit( queue_job_cmd, submit_job_msg, tmpdir, + monkeypatch, ): p_run_dir = tmpdir.ensure_dir("run_dir") p_results_dir = tmpdir.ensure_dir("results_dir") @@ -200,10 +202,12 @@ def test_run_submit( Path(str(p_run_dir), "SalishSeaNEMO.sh"), ) m_sj.return_value = submit_job_msg - with patch("salishsea_cmd.run.SYSTEM", system): - submit_job_msg = salishsea_cmd.run.run( - Path("SalishSea.yaml"), Path(str(p_results_dir)) - ) + monkeypatch.setattr(salishsea_cmd.run, "SYSTEM", system) + + submit_job_msg = salishsea_cmd.run.run( + Path("SalishSea.yaml"), Path(str(p_results_dir)) + ) + m_sj.assert_called_once_with( Path(str(p_run_dir), "SalishSeaNEMO.sh"), queue_job_cmd, waitjob="0" ) @@ -220,8 +224,8 @@ def test_run_submit( (True, 4, "delta", "qsub -q mpi", "43.admin.default.domain"), (False, 0, "graham", "sbatch", "Submitted batch job 43"), (True, 4, "graham", "sbatch", "Submitted batch job 43"), - (False, 0, "salish", "qsub", "43.master"), - (True, 4, "salish", "qsub", "43.master"), + (False, 0, "salish", "bash", "bash run_dir/SalishSeaNEMO.sh started"), + (True, 4, "salish", "bash", "bash run_dir/SalishSeaNEMO.sh started"), (False, 0, "sigma", "qsub -q mpi", "43.admin.default.domain"), (True, 4, "sigma", "qsub -q mpi", "43.admin.default.domain"), (False, 0, "sockeye", "sbatch", "Submitted batch job 43"), @@ -386,8 +390,8 @@ def test_run_no_submit_w_separate_deflate( (True, 4, "delta", "qsub -q mpi", "43.admin.default.domain"), (False, 0, "graham", "sbatch", "Submitted batch job 43"), (True, 4, "graham", "sbatch", "Submitted batch job 43"), - (False, 0, "salish", "qsub", "43.master"), - (True, 4, "salish", "qsub", "43.master"), + (False, 0, "salish", "bash", "bash run_dir/SalishSeaNEMO.sh started"), + (True, 4, "salish", "bash", "bash run_dir/SalishSeaNEMO.sh started"), (False, 0, "sigma", "qsub -q mpi", "43.admin.default.domain"), (True, 4, "sigma", "qsub -q mpi", "43.admin.default.domain"), (False, 0, "sockeye", "sbatch", "Submitted batch job 43"), @@ -520,8 +524,8 @@ def test_run_separate_deflate( ("Submitted batch job 43", "Submitted batch job 44"), "Submitted batch job 43", ), - (False, 0, "salish", "qsub", ("43.master", "44.master"), "43.master"), - (True, 4, "salish", "qsub", ("43.master", "44.master"), "43.master"), + (False, 0, "salish", "bash", ("43.master", "44.master"), "43.master"), + (True, 4, "salish", "bash", ("43.master", "44.master"), "43.master"), ( False, 0, @@ -748,8 +752,8 @@ def test_segmented_run_restart_dirs( ("Submitted batch job 43", "Submitted batch job 44"), "Submitted batch job 43", ), - (False, 0, "salish", "qsub", ("43.master", "44.master"), "43.master"), - (True, 4, "salish", "qsub", ("43.master", "44.master"), "43.master"), + (False, 0, "salish", "bash", ("43.master", "44.master"), "43.master"), + (True, 4, "salish", "bash", ("43.master", "44.master"), "43.master"), ( False, 0, @@ -1824,48 +1828,95 @@ def test_build_tmp_run_dir_separate_deflate( assert batch_file == Path(str(p_run_dir)) / "SalishSeaNEMO.sh" -@patch("salishsea_cmd.run.subprocess.run") -@pytest.mark.parametrize( - "queue_job_cmd, depend_flag, depend_option, submit_job_msg", - [ - ("sbatch", "-d", "afterok", "Submitted batch job 43"), - ("qsub", "-W", "depend=afterok", "43.orca2.ibb"), - ("qsub -q mpi", "-W", "depend=afterok", "43.admin.default.domain"), - ], -) class TestSubmitJob: """Unit tests for _submit_job() function.""" - def test_submit_job( - self, m_run, queue_job_cmd, depend_flag, depend_option, submit_job_msg - ): + @pytest.mark.parametrize( + "queue_job_cmd, msg", + ( + ("bash", "bash run_dir/SalishSeaNEMO.sh started"), # salish + ("sbatch", "Submitted batch job 43"), # sockeye & Alliance HPC clusters + ("qsub", "43.orca2.ibb"), # orcinus + ("qsub -q mpi", "43.admin.default.domain"), # optimum + ), + ) + def test_submit_job(self, queue_job_cmd, msg, monkeypatch): + def mock_subprocess_Popen(cmd, start_new_session): + pass + + monkeypatch.setattr(subprocess, "Popen", mock_subprocess_Popen) + + def mock_subprocess_run(cmd, check, universal_newlines, stdout): + return subprocess.CompletedProcess(cmd, 0, msg) + + monkeypatch.setattr(subprocess, "run", mock_subprocess_run) + submit_job_msg = salishsea_cmd.run._submit_job( Path("run_dir", "SalishSeaNEMO.sh"), queue_job_cmd, "0" ) - m_run.assert_called_once_with( - shlex.split(f"{queue_job_cmd} {Path('run_dir')}/SalishSeaNEMO.sh"), - check=True, - universal_newlines=True, - stdout=subprocess.PIPE, - ) - assert submit_job_msg == submit_job_msg - def test_submit_job_w_waitjob( - self, m_run, queue_job_cmd, depend_flag, depend_option, submit_job_msg + assert submit_job_msg == msg + + @pytest.mark.parametrize( + "queue_job_cmd, depend_flag, depend_option, msg", + [ + # sockeye & Alliance HPC clusters + ( + "sbatch", + "-d", + "afterok", + "Submitted batch job 43", + ), + # sockeye & Alliance HPC clusters + ( + "qsub", + "-W", + "depend=afterok", + "43.orca2.ibb", + ), + # optimum + ( + "qsub -q mpi", + "-W", + "depend=afterok", + "43.admin.default.domain", + ), + ], + ) + def test_submit_job_with_waitjob( + self, + queue_job_cmd, + depend_flag, + depend_option, + msg, + monkeypatch, ): + def mock_subprocess_run(cmd, check, universal_newlines, stdout): + return subprocess.CompletedProcess(cmd, 0, msg) + + monkeypatch.setattr(subprocess, "run", mock_subprocess_run) + submit_job_msg = salishsea_cmd.run._submit_job( Path("run_dir", "SalishSeaNEMO.sh"), queue_job_cmd, 42 ) - m_run.assert_called_once_with( - shlex.split( - f"{queue_job_cmd} {depend_flag} {depend_option}:42 {Path('run_dir')}/SalishSeaNEMO.sh" - ), - check=True, - universal_newlines=True, - stdout=subprocess.PIPE, - ) + assert submit_job_msg == submit_job_msg + def test_no_waitjob_for_bash_submit(self, caplog): + caplog.set_level(logging.DEBUG) + + with pytest.raises(SystemExit) as exc: + salishsea_cmd.run._submit_job( + Path("run_dir", "SalishSeaNEMO.sh"), "bash", "43" + ) + + assert exc.value.code == 2 + assert caplog.records[0].levelname == "ERROR" + expected = ( + "dependent jobs are not available for systems that launch jobs with bash" + ) + assert caplog.records[0].message == expected + @patch("salishsea_cmd.run.log", autospec=True) @patch("salishsea_cmd.run.subprocess.run")