Skip to content

Commit 7b06a22

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 7b06a22

File tree

2 files changed

+86
-3
lines changed

2 files changed

+86
-3
lines changed

snakemake_executor_plugin_slurm/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,7 +1590,7 @@ def get_account_arg(self, job: JobExecutorInterface):
15901590
# split the account upon ',' and whitespace, to allow
15911591
# multiple accounts being given
15921592
accounts = [
1593-
a for a in re.split(r"[,\s]+", job.resources.slurm_account) if a
1593+
a for a in re.split(r"[,\s]+", str(job.resources.slurm_account)) if a
15941594
]
15951595
for account in accounts:
15961596
# here, we check whether the given or guessed account is valid
@@ -1602,15 +1602,15 @@ def get_account_arg(self, job: JobExecutorInterface):
16021602
# contain build-in shell commands - see issue #354
16031603
for account in accounts:
16041604
test_account(account, self.logger)
1605-
yield f" -A {shlex.quote(account)}"
1605+
yield f" -A {shlex.quote(str(account))}"
16061606
else:
16071607
if self._fallback_account_arg is None:
16081608
self.logger.warning("No SLURM account given, trying to guess.")
16091609
account = get_account(self.logger)
16101610
if account:
16111611
self.logger.warning(f"Guessed SLURM account: {account}")
16121612
test_account(f"{account}", self.logger)
1613-
self._fallback_account_arg = f" -A {shlex.quote(account)}"
1613+
self._fallback_account_arg = f" -A {shlex.quote(str(account))}"
16141614
else:
16151615
self.logger.warning(
16161616
"Unable to guess SLURM account. Trying to proceed without."

tests/test_account.py

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

0 commit comments

Comments
 (0)