Skip to content

Commit 08a867a

Browse files
cmeestersDrYak
andauthored
fix: skip account setting upon user request (#224)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Users can now opt-out from automatic account specification during job submission using a new configuration flag, offering greater flexibility for environments with multiple accounts or enforced defaults. - **Documentation** - Enhanced guidance on SLURM account management, including advice on explicitly setting accounts when necessary and details on overriding default behaviors with a new flag. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: DrYak <[email protected]>
1 parent c166eb3 commit 08a867a

File tree

2 files changed

+36
-12
lines changed

2 files changed

+36
-12
lines changed

docs/further.md

+3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ To specify them at the command line, define them as default resources:
1818
$ snakemake --executor slurm --default-resources slurm_account=<your SLURM account> slurm_partition=<your SLURM partition>
1919
```
2020

21+
The plugin does its best to _guess_ your account. That might not be possible. Particularly, when dealing with several SLURM accounts, users ought to set them per workflow.
22+
Some clusters, however, have a pre-defined default per user and _do not_ allow users to set their account or partition. The plugin will always attempt to set an account. To override this behavior, the `--slurm-no-account` flag can be used.
23+
2124
If individual rules require e.g. a different partition, you can override
2225
the default per rule:
2326

snakemake_executor_plugin_slurm/__init__.py

+33-12
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,15 @@ class ExecutorSettings(ExecutorSettingsBase):
8585
"required": False,
8686
},
8787
)
88+
no_account: bool = field(
89+
default=False,
90+
metadata={
91+
"help": "Do not use any account for submission. "
92+
"This flag has no effect, if not set.",
93+
"env_var": False,
94+
"required": False,
95+
},
96+
)
8897

8998

9099
# Required:
@@ -213,7 +222,9 @@ def run_job(self, job: JobExecutorInterface):
213222
f"--comment '{comment_str}'"
214223
)
215224

216-
call += self.get_account_arg(job)
225+
if not self.workflow.executor_settings.no_account:
226+
call += self.get_account_arg(job)
227+
217228
call += self.get_partition_arg(job)
218229

219230
if self.workflow.executor_settings.requeue:
@@ -634,35 +645,45 @@ def test_account(self, account):
634645
"""
635646
tests whether the given account is registered, raises an error, if not
636647
"""
637-
cmd = f'sacctmgr -n -s list user "{os.environ["USER"]}" format=account%256'
648+
cmd = "sshare -U --format Account --noheader"
638649
try:
639650
accounts = subprocess.check_output(
640651
cmd, shell=True, text=True, stderr=subprocess.PIPE
641652
)
642653
except subprocess.CalledProcessError as e:
643-
sacctmgr_report = (
644-
"Unable to test the validity of the given or guessed "
645-
f"SLURM account '{account}' with sacctmgr: {e.stderr}."
654+
sshare_report = (
655+
"Unable to test the validity of the given or guessed"
656+
f" SLURM account '{account}' with sshare: {e.stderr}."
646657
)
658+
accounts = ""
659+
660+
if not accounts.strip():
661+
cmd = f'sacctmgr -n -s list user "{os.environ["USER"]}" format=account%256'
647662
try:
648-
cmd = "sshare -U --format Account --noheader"
649663
accounts = subprocess.check_output(
650664
cmd, shell=True, text=True, stderr=subprocess.PIPE
651665
)
652-
except subprocess.CalledProcessError as e2:
653-
sshare_report = (
654-
"Unable to test the validity of the given or guessed"
655-
f" SLURM account '{account}' with sshare: {e2.stderr}."
666+
except subprocess.CalledProcessError as e:
667+
sacctmgr_report = (
668+
"Unable to test the validity of the given or guessed "
669+
f"SLURM account '{account}' with sacctmgr: {e.stderr}."
656670
)
657671
raise WorkflowError(
658-
f"The 'sacctmgr' reported: '{sacctmgr_report}' "
659-
f"and likewise 'sshare' reported: '{sshare_report}'."
672+
f"The 'sshare' reported: '{sshare_report}' "
673+
f"and likewise 'sacctmgr' reported: '{sacctmgr_report}'."
660674
)
661675

662676
# The set() has been introduced during review to eliminate
663677
# duplicates. They are not harmful, but disturbing to read.
664678
accounts = set(_.strip() for _ in accounts.split("\n") if _)
665679

680+
if not accounts:
681+
self.logger.warning(
682+
f"Both 'sshare' and 'sacctmgr' returned empty results for account "
683+
f"'{account}'. Proceeding without account validation."
684+
)
685+
return ""
686+
666687
if account not in accounts:
667688
raise WorkflowError(
668689
f"The given account {account} appears to be invalid. Available "

0 commit comments

Comments
 (0)