Skip to content

Commit 67cd465

Browse files
committed
fix(slurm): handle integer slurm_account values from YAML parsing
Snakemake's YAML parser automatically converts numeric-looking strings (e.g. "123456789") to integers when populating job.resources. This caused account.lower() in test_account() to fail since int has no lower() method. Convert slurm_account and account values to strings before use in re.split() and shlex.quote() calls. Fixes the issue where users specifying slurm_account as a numeric string in their YAML profile would get an AttributeError.
1 parent 7fa975f commit 67cd465

2 files changed

Lines changed: 80 additions & 1 deletion

File tree

snakemake_executor_plugin_slurm/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1589,8 +1589,10 @@ def get_account_arg(self, job: JobExecutorInterface):
15891589
if job.resources.get("slurm_account"):
15901590
# split the account upon ',' and whitespace, to allow
15911591
# multiple accounts being given
1592+
# Note: YAML parsing may convert numeric strings (e.g. "123456") to int.
1593+
# Ensure we always work with strings for re.split and shlex.quote.
15921594
accounts = [
1593-
a for a in re.split(r"[,\s]+", job.resources.slurm_account) if a
1595+
a for a in re.split(r"[,\s]+", str(job.resources.slurm_account)) if a
15941596
]
15951597
for account in accounts:
15961598
# here, we check whether the given or guessed account is valid

tests/test_account.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
"""Unit tests for get_account_arg() method."""
2+
3+
from unittest.mock import MagicMock, patch
4+
5+
from snakemake_executor_plugin_slurm import Executor
6+
7+
8+
class _Resources(dict):
9+
"""Dict-like resources with attribute access for known keys only."""
10+
11+
def __getattr__(self, name):
12+
try:
13+
return self[name]
14+
except KeyError as e:
15+
raise AttributeError(name) from e
16+
17+
18+
def _make_mock_job(rule_name="myrule", name=None, wildcards=None, jobid=1, is_group=False, **resources):
19+
"""Return a minimal mock job compatible with get_account_arg."""
20+
mock_resources = _Resources(resources)
21+
22+
mock_rule = MagicMock()
23+
mock_rule.name = rule_name
24+
25+
job = MagicMock()
26+
job.resources = mock_resources
27+
job.rule = mock_rule
28+
job.name = name if name is not None else rule_name
29+
job.wildcards = wildcards if wildcards is not None else {}
30+
job.is_group.return_value = is_group
31+
job.threads = resources.get("threads", 1)
32+
job.jobid = jobid
33+
return job
34+
35+
36+
class TestGetAccountArg:
37+
"""Tests for get_account_arg() method handling string and integer accounts."""
38+
39+
def _make_executor_stub(self):
40+
"""Return a minimal Executor stub."""
41+
executor = Executor.__new__(Executor)
42+
executor.logger = MagicMock()
43+
executor._fallback_account_arg = None
44+
return executor
45+
46+
# Patches prevent actual SLURM command execution (sacct/sacctmgr).
47+
# - get_account() would call 'sacct' to guess account from recent jobs.
48+
# - test_account() would call 'sacctmgr' or 'sshare' to validate account.
49+
# Mocks allow testing without a live SLURM cluster.
50+
@patch("snakemake_executor_plugin_slurm.get_account")
51+
@patch("snakemake_executor_plugin_slurm.test_account")
52+
def test_string_account(self, mock_test_account, mock_get_account, tmp_path):
53+
"""String account values work correctly."""
54+
executor = self._make_executor_stub()
55+
job = _make_mock_job(slurm_account="123456789")
56+
result = next(executor.get_account_arg(job))
57+
assert result == " -A 123456789"
58+
mock_test_account.assert_called_with("123456789", executor.logger)
59+
60+
@patch("snakemake_executor_plugin_slurm.get_account")
61+
@patch("snakemake_executor_plugin_slurm.test_account")
62+
def test_integer_account(self, mock_test_account, mock_get_account, tmp_path):
63+
"""Integer account values (from YAML parsing) work correctly."""
64+
executor = self._make_executor_stub()
65+
job = _make_mock_job(slurm_account=123456789)
66+
result = next(executor.get_account_arg(job))
67+
assert result == " -A 123456789"
68+
mock_test_account.assert_called_with("123456789", executor.logger)
69+
70+
@patch("snakemake_executor_plugin_slurm.get_account")
71+
@patch("snakemake_executor_plugin_slurm.test_account")
72+
def test_multiple_accounts_string(self, mock_test_account, mock_get_account, tmp_path):
73+
"""Multiple string accounts work correctly."""
74+
executor = self._make_executor_stub()
75+
job = _make_mock_job(slurm_account="acc1,acc2 acc3")
76+
results = list(executor.get_account_arg(job))
77+
assert results == [" -A acc1", " -A acc2", " -A acc3"]

0 commit comments

Comments
 (0)