-
Notifications
You must be signed in to change notification settings - Fork 189
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
support 'UNLIMITED' and 'NOT_SET' slurm times #3586
Conversation
requested walltimes in slurm can be 'UNLIMITED' and 'NOT_SET'. now supporting these in the slurm plugin (& added tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ltalirz aside from the suggestion to change the get_job
function to just creating a mapping once and indexing that, this is good for me. However, I would like @greschd and @giovannipizzi to weigh in on the final choice of values for UNLIMITED
and NOT_SET
@@ -46,43 +54,46 @@ def test_parse_common_joblist_output(self): | |||
|
|||
job_list = scheduler._parse_joblist_output(retval, stdout, stderr) | |||
|
|||
def get_job(id): | |||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably be a bit easier and more efficient to just turn job_list
into a mapping on job_id
and then index that in the tests instead of doing get_job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks very good to me!
Looks good to me, too. Thanks for implementing this! |
Fixes #3415
requested walltimes in slurm can be 'UNLIMITED' and 'NOT_SET'.
now supporting these in the slurm plugin (& added tests).