search

snakemake / snakemake-executor-plugin-slurm

A Snakemake executor plugin for submitting jobs to a SLURM cluster
MIT License
18 stars 19 forks source link

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

Closed dnerini closed 9 months ago

dnerini commented 9 months ago

Closes #25

Tested with slurm 20.02.07

Not a great contribution in terms of code readability, but it'd ensure that the plugin works with older slurm versions.

fgvieira commented 9 months ago

slurm 2.11.0 is more than 3 years old. Wouldn't it be better to update it?

dnerini commented 9 months ago

I agree, it would be great, but unfortunately that is not an option in my case. I'm happy to keep using my fork. I just thought I would submit the PR in case someone else is facing the same issue.

cmeesters commented 9 months ago

Yep, and I would support this PR rather than lose the idea. I would also hate to require from you @dnerini to update your fork, whenever a relevant feature or fix is merged upstream ... and I understand that some admins are very reluctant to update their slurm. Although they really should due to security fixes.

Please run black on your code and add the comment.

dnerini commented 9 months ago

thanks @cmeesters for the feedback. I reformatted the code, added a comment, and changed to local time using datetime.now() (although it shouldn't make a difference for the purpose introduced in #9).