From 0a196287a28e904cc4375bc88e554883057e1a5b Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Fri, 7 Mar 2025 17:44:18 +0100 Subject: [PATCH 1/8] fix: a leading slash is escaped when constructing the wildcard combi for a logdir file name --- snakemake_executor_plugin_slurm/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snakemake_executor_plugin_slurm/__init__.py b/snakemake_executor_plugin_slurm/__init__.py index e17d72b0..5040d970 100644 --- a/snakemake_executor_plugin_slurm/__init__.py +++ b/snakemake_executor_plugin_slurm/__init__.py @@ -180,7 +180,7 @@ def run_job(self, job: JobExecutorInterface): group_or_rule = f"group_{job.name}" if job.is_group() else f"rule_{job.name}" try: - wildcard_str = "_".join(job.wildcards) if job.wildcards else "" + wildcard_str = "_".join(job.wildcards).replace("/", "_") if job.wildcards else "" except AttributeError: wildcard_str = "" From 9387b8d57d2b824f7dc6fec49db22c42546c5e2d Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Fri, 7 Mar 2025 17:51:58 +0100 Subject: [PATCH 2/8] fix: forgot linting --- snakemake_executor_plugin_slurm/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/snakemake_executor_plugin_slurm/__init__.py b/snakemake_executor_plugin_slurm/__init__.py index 5040d970..534a047a 100644 --- a/snakemake_executor_plugin_slurm/__init__.py +++ b/snakemake_executor_plugin_slurm/__init__.py @@ -180,7 +180,9 @@ def run_job(self, job: JobExecutorInterface): group_or_rule = f"group_{job.name}" if job.is_group() else f"rule_{job.name}" try: - wildcard_str = "_".join(job.wildcards).replace("/", "_") if job.wildcards else "" + wildcard_str = ( + "_".join(job.wildcards).replace("/", "_") if job.wildcards else "" + ) except AttributeError: wildcard_str = "" From d69f9651b219218d10750946f305ba495f0321a8 Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Fri, 7 Mar 2025 17:56:14 +0100 Subject: [PATCH 3/8] fix: update of pyproject.toml needs to reflect update of the jobstep plugin --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 1b9e9096..26f98cf5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,7 @@ keywords = ["snakemake", "plugin", "executor", "cluster", "slurm"] python = "^3.11" snakemake-interface-common = "^1.13.0" snakemake-interface-executor-plugins = "^9.1.1" -snakemake-executor-plugin-slurm-jobstep = "^0.2.0" +snakemake-executor-plugin-slurm-jobstep = "^0.3.0" throttler = "^1.2.2" [tool.poetry.group.dev.dependencies] From cdb3307194ca2b6d59a97de943a519e2715a3159 Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Fri, 7 Mar 2025 18:08:23 +0100 Subject: [PATCH 4/8] fix: reflecting update in jobstep plugin --- snakemake_executor_plugin_slurm/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/snakemake_executor_plugin_slurm/__init__.py b/snakemake_executor_plugin_slurm/__init__.py index 534a047a..b9d0561c 100644 --- a/snakemake_executor_plugin_slurm/__init__.py +++ b/snakemake_executor_plugin_slurm/__init__.py @@ -26,7 +26,7 @@ JobExecutorInterface, ) from snakemake_interface_common.exceptions import WorkflowError -from snakemake_executor_plugin_slurm_jobstep import get_cpus_per_task +from snakemake_executor_plugin_slurm_jobstep import get_cpu_setting from .utils import delete_slurm_environment, delete_empty_dirs @@ -261,7 +261,7 @@ def run_job(self, job: JobExecutorInterface): "Probably not what you want." ) - call += f" --cpus-per-task={get_cpus_per_task(job)}" + call += f" --cpus-per-task={get_cpu_setting(job)}" if job.resources.get("slurm_extra"): self.check_slurm_extra(job) From 3ebb2dcaaff7b9613282f443476ac34e6eefbba2 Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Tue, 11 Mar 2025 13:53:14 +0100 Subject: [PATCH 5/8] feat: added test cases --- tests/tests.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/tests.py b/tests/tests.py index bba5f398..c7be3cb5 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -110,3 +110,47 @@ def test_both_gres_and_gpu_set(self, mock_job): WorkflowError, match="GRES and GPU are set. Please only set one of them." ): set_gres_string(job) + + +class TestWildcardsWithSlashes(snakemake.common.tests.TestWorkflowsLocalStorageBase): + """Test handling of wildcards with slashes to ensure log directories are correctly constructed.""" + + __test__ = True + + def get_executor(self) -> str: + return "slurm" + + def get_executor_settings(self) -> Optional[ExecutorSettingsBase]: + return ExecutorSettings(logdir="test_logdir") + + def test_wildcard_slash_replacement(self): + """Test that slashes in wildcards are correctly replaced with underscores in log directory paths.""" + + # Create a mock job with wildcards containing slashes + class MockJob: + def __init__(self): + self.name = "test_job" + self.wildcards = ["/leading_slash", "middle/slash", "trailing/"] + self.is_group = lambda: False + + # Create an executor instance + executor = Executor() + executor.workflow = snakemake.common.tests.DummyWorkflow() + executor.workflow.executor_settings = self.get_executor_settings() + executor.slurm_logdir = os.path.abspath("test_logdir") + + # Manually call the wildcard sanitization logic from run_job method + try: + wildcard_str = ( + "_".join(MockJob().wildcards).replace("/", "_") + if MockJob().wildcards + else "" + ) + except AttributeError: + wildcard_str = "" + + # Assert that slashes are correctly replaced with underscores + assert wildcard_str == "_leading_slash_middle_slash_trailing_" + + # Verify no slashes remain in the wildcard string + assert "/" not in wildcard_str From b6405283609da1a8c08277c2abe0d949af54021c Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Tue, 11 Mar 2025 13:57:24 +0100 Subject: [PATCH 6/8] fix: fix for linting --- tests/tests.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/tests.py b/tests/tests.py index c7be3cb5..3defd702 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1,10 +1,11 @@ +import os from typing import Optional import snakemake.common.tests from snakemake_interface_executor_plugins.settings import ExecutorSettingsBase from unittest.mock import MagicMock import pytest -from snakemake_executor_plugin_slurm import ExecutorSettings +from snakemake_executor_plugin_slurm import ExecutorSettings, Executor from snakemake_executor_plugin_slurm.utils import set_gres_string from snakemake_interface_common.exceptions import WorkflowError @@ -113,7 +114,10 @@ def test_both_gres_and_gpu_set(self, mock_job): class TestWildcardsWithSlashes(snakemake.common.tests.TestWorkflowsLocalStorageBase): - """Test handling of wildcards with slashes to ensure log directories are correctly constructed.""" + """ + Test handling of wildcards with slashes to ensure log directories are + correctly constructed. + """ __test__ = True @@ -124,7 +128,10 @@ def get_executor_settings(self) -> Optional[ExecutorSettingsBase]: return ExecutorSettings(logdir="test_logdir") def test_wildcard_slash_replacement(self): - """Test that slashes in wildcards are correctly replaced with underscores in log directory paths.""" + """ + Test that slashes in wildcards are correctly replaced with + underscores in log directory paths. + """ # Create a mock job with wildcards containing slashes class MockJob: From ccc096d397af0514cb527f8012a05cf011118d33 Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Tue, 11 Mar 2025 14:20:12 +0100 Subject: [PATCH 7/8] fix: corrected test case + less waiting for job status updates for slurm executor --- tests/tests.py | 43 ++++++++++++++----------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/tests/tests.py b/tests/tests.py index 3defd702..33f538b4 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -17,12 +17,12 @@ def get_executor(self) -> str: return "slurm" def get_executor_settings(self) -> Optional[ExecutorSettingsBase]: - return ExecutorSettings() + return ExecutorSettings(init_seconds_before_status_checks=1) class TestWorkflowsRequeue(TestWorkflows): def get_executor_settings(self) -> Optional[ExecutorSettingsBase]: - return ExecutorSettings(requeue=True) + return ExecutorSettings(requeue=True, init_seconds_before_status_checks=1) class TestGresString: @@ -125,7 +125,9 @@ def get_executor(self) -> str: return "slurm" def get_executor_settings(self) -> Optional[ExecutorSettingsBase]: - return ExecutorSettings(logdir="test_logdir") + return ExecutorSettings( + logdir="test_logdir", init_seconds_before_status_checks=1 + ) def test_wildcard_slash_replacement(self): """ @@ -133,31 +135,14 @@ def test_wildcard_slash_replacement(self): underscores in log directory paths. """ - # Create a mock job with wildcards containing slashes - class MockJob: - def __init__(self): - self.name = "test_job" - self.wildcards = ["/leading_slash", "middle/slash", "trailing/"] - self.is_group = lambda: False - - # Create an executor instance - executor = Executor() - executor.workflow = snakemake.common.tests.DummyWorkflow() - executor.workflow.executor_settings = self.get_executor_settings() - executor.slurm_logdir = os.path.abspath("test_logdir") - - # Manually call the wildcard sanitization logic from run_job method - try: - wildcard_str = ( - "_".join(MockJob().wildcards).replace("/", "_") - if MockJob().wildcards - else "" - ) - except AttributeError: - wildcard_str = "" + # Just test the wildcard sanitization logic directly + wildcards = ["/leading_slash", "middle/slash", "trailing/"] + + # This is the actual logic from the Executor.run_job method + wildcard_str = "_".join(wildcards).replace("/", "_") if wildcards else "" - # Assert that slashes are correctly replaced with underscores - assert wildcard_str == "_leading_slash_middle_slash_trailing_" + # Assert that slashes are correctly replaced with underscores + assert wildcard_str == "_leading_slash_middle_slash_trailing_" - # Verify no slashes remain in the wildcard string - assert "/" not in wildcard_str + # Verify no slashes remain in the wildcard string + assert "/" not in wildcard_str From 2d5d7e22a04a665bc9f0abcbffda7cc3ecaae31d Mon Sep 17 00:00:00 2001 From: Christian Meesters Date: Tue, 11 Mar 2025 14:26:14 +0100 Subject: [PATCH 8/8] fix: removed unused imports from tests.py --- tests/tests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/tests.py b/tests/tests.py index 33f538b4..63a7dcf3 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1,11 +1,10 @@ -import os from typing import Optional import snakemake.common.tests from snakemake_interface_executor_plugins.settings import ExecutorSettingsBase from unittest.mock import MagicMock import pytest -from snakemake_executor_plugin_slurm import ExecutorSettings, Executor +from snakemake_executor_plugin_slurm import ExecutorSettings from snakemake_executor_plugin_slurm.utils import set_gres_string from snakemake_interface_common.exceptions import WorkflowError