-
Notifications
You must be signed in to change notification settings - Fork 146
support older slurm job_state schema #1137
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 older slurm job_state schema #1137
Conversation
|
@amirafzali has exported this pull request. If you are a Meta employee, you can view the originating Diff in D83690140. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1137 +/- ##
=======================================
Coverage 91.65% 91.66%
=======================================
Files 83 83
Lines 6422 6427 +5
=======================================
+ Hits 5886 5891 +5
Misses 536 536
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
please fix |
hm it's a loaded json (https://github.com/meta-pytorch/torchx/blob/main/torchx/schedulers/slurm_scheduler.py#L654-L663), do we have an existing type for this? |
pyre doesn't like it when public functions use |
Summary: With SLURM <= 23.02, job_state was a string and not a list. Even with the list we only use the first element. Added support for string type https://slurm.schedmd.com/archive/slurm-23.02-latest/rest_api.html#v0_0_39_job_state Reviewed By: kiukchung Differential Revision: D83690140
f0bba86 to
5cfebe7
Compare
Summary:
With SLURM <= 23.02, job_state was a string and not a list. Even with the list we only use the first element.
Added support for string type
https://slurm.schedmd.com/archive/slurm-23.02-latest/rest_api.html#v0_0_39_job_state
Differential Revision: D83690140