Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use sacct syntax that is compatible with slurm < 20.11.0 #26

Merged
merged 6 commits into from
Feb 14, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion snakemake_executor_plugin_slurm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import os
import subprocess
import time
from datetime import datetime, timedelta
from typing import List, Generator
import uuid
from snakemake_interface_executor_plugins.executors.base import SubmittedJobInfo
Expand Down Expand Up @@ -199,14 +200,23 @@ async def check_active_jobs(
active_jobs_ids = {job_info.external_jobid for job_info in active_jobs}
active_jobs_seen_by_sacct = set()

# We use this sacct syntax for argument 'starttime' to keep it compatible
# with slurm < 20.11
sacct_starttime = f"{datetime.now() - timedelta(days=2):%Y-%m-%dT%H:00}"
# previously we had
# f"--starttime now-2days --endtime now --name {self.run_uuid}"
# in line 218 - once v20.11 is definitively not in use any more,
# the more readable version ought to be re-adapted

# this code is inspired by the snakemake profile:
# https://github.com/Snakemake-Profiles/slurm
for i in range(status_attempts):
async with self.status_rate_limiter:
(status_of_jobs, sacct_query_duration) = await self.job_stati(
# -X: only show main job, no substeps
f"sacct -X --parsable2 --noheader --format=JobIdRaw,State "
f"--starttime now-2days --endtime now --name {self.run_uuid}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please just out comment this line and add an appropriate comment: While ensuring backwards compatibility is worth the change, eventually, such clusters will be out phased for good, and we will hardly know why the following line has been introduced at all.

f"--starttime {sacct_starttime} "
f"--endtime now --name {self.run_uuid}"
)
if status_of_jobs is None and sacct_query_duration is None:
self.logger.debug(f"could not check status of job {self.run_uuid}")
Expand Down
Loading