fix: Use sacct syntax that is compatible with slurm < 20.11.0 #26
Conversation
|
slurm |
|
I agree, it would be great, but unfortunately that is not an option in my case. |
|
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. |
|
thanks @cmeesters for the feedback. I reformatted the code, added a comment, and changed to local time using |
wanted to keep original code version (outcommented) present
fixed trailing white space
| @@ -205,7 +206,8 @@ async def check_active_jobs( | |||
| (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}" | |||
There was a problem hiding this comment.
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.
🤖 I have created a release *beep* *boop* --- ## [0.3.1](v0.3.0...v0.3.1) (2024-02-14) ### Bug Fixes * Use sacct syntax that is compatible with slurm < 20.11.0 ([#26](#26)) ([c1591ff](c1591ff)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.