Skip to content

Commit 61de847

Browse files
authored
fix: logdir misconstruct when leading slash in wildcard (#220)
This is a minor fix, which addresses issue #214: A slurm logdirectory will start with `/` if a wildcard starts with `/`. After discussing with @johanneskoester , allowing for slashes in wildcards should not be an issue. Hence, the escape in this PR. Also, snakemake/snakemake#3318 can be considered solved. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced functionality for requesting GPU resources in SLURM jobs, with detailed documentation on methods for specifying GPUs and associated CPU settings. - **Bug Fixes** - Improved handling of job resource specifications to enforce clarity between GRES and GPU inputs, preventing potential conflicts. - **Dependencies** - Updated the version of the `snakemake-executor-plugin-slurm-jobstep` dependency from `^0.2.0` to `^0.3.0`, which may include enhancements and bug fixes. - **Documentation** - Added a new section on "GPU Jobs" outlining how to request GPU resources with examples and clarifications on resource specifications. - **Tests** - Removed unnecessary import from test files, streamlining the test code without affecting functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 66dcdcf commit 61de847

File tree

2 files changed

+41
-4
lines changed

2 files changed

+41
-4
lines changed

snakemake_executor_plugin_slurm/__init__.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ def run_job(self, job: JobExecutorInterface):
180180
group_or_rule = f"group_{job.name}" if job.is_group() else f"rule_{job.name}"
181181

182182
try:
183-
wildcard_str = "_".join(job.wildcards) if job.wildcards else ""
183+
wildcard_str = (
184+
"_".join(job.wildcards).replace("/", "_") if job.wildcards else ""
185+
)
184186
except AttributeError:
185187
wildcard_str = ""
186188

@@ -264,10 +266,10 @@ def run_job(self, job: JobExecutorInterface):
264266
"specified. Assuming 'tasks_per_node=1'."
265267
"Probably not what you want."
266268
)
269+
267270
# we need to set cpus-per-task OR cpus-per-gpu, the function
268271
# will return a string with the corresponding value
269272
call += f" {get_cpu_setting(job, gpu_job)}"
270-
271273
if job.resources.get("slurm_extra"):
272274
self.check_slurm_extra(job)
273275
call += f" {job.resources.slurm_extra}"

tests/tests.py

+37-2
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ def get_executor(self) -> str:
1616
return "slurm"
1717

1818
def get_executor_settings(self) -> Optional[ExecutorSettingsBase]:
19-
return ExecutorSettings()
19+
return ExecutorSettings(init_seconds_before_status_checks=1)
2020

2121

2222
class TestWorkflowsRequeue(TestWorkflows):
2323
def get_executor_settings(self) -> Optional[ExecutorSettingsBase]:
24-
return ExecutorSettings(requeue=True)
24+
return ExecutorSettings(requeue=True, init_seconds_before_status_checks=1)
2525

2626

2727
class TestGresString:
@@ -110,3 +110,38 @@ def test_both_gres_and_gpu_set(self, mock_job):
110110
WorkflowError, match="GRES and GPU are set. Please only set one of them."
111111
):
112112
set_gres_string(job)
113+
114+
115+
class TestWildcardsWithSlashes(snakemake.common.tests.TestWorkflowsLocalStorageBase):
116+
"""
117+
Test handling of wildcards with slashes to ensure log directories are
118+
correctly constructed.
119+
"""
120+
121+
__test__ = True
122+
123+
def get_executor(self) -> str:
124+
return "slurm"
125+
126+
def get_executor_settings(self) -> Optional[ExecutorSettingsBase]:
127+
return ExecutorSettings(
128+
logdir="test_logdir", init_seconds_before_status_checks=1
129+
)
130+
131+
def test_wildcard_slash_replacement(self):
132+
"""
133+
Test that slashes in wildcards are correctly replaced with
134+
underscores in log directory paths.
135+
"""
136+
137+
# Just test the wildcard sanitization logic directly
138+
wildcards = ["/leading_slash", "middle/slash", "trailing/"]
139+
140+
# This is the actual logic from the Executor.run_job method
141+
wildcard_str = "_".join(wildcards).replace("/", "_") if wildcards else ""
142+
143+
# Assert that slashes are correctly replaced with underscores
144+
assert wildcard_str == "_leading_slash_middle_slash_trailing_"
145+
146+
# Verify no slashes remain in the wildcard string
147+
assert "/" not in wildcard_str

0 commit comments

Comments
 (0)