-
Notifications
You must be signed in to change notification settings - Fork 356
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: partially scheduled k8s jobs should display as queued #9468
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9468 +/- ##
=======================================
Coverage 49.28% 49.28%
=======================================
Files 1242 1242
Lines 161441 161442 +1
Branches 2868 2868
=======================================
+ Hits 79564 79567 +3
+ Misses 81705 81703 -2
Partials 172 172
Flags with carried forward coverage won't be shown. Click here to find out more.
|
t.Error("state went to PULLING or beyond when all pods could not have been scheduled") | ||
t.FailNow() | ||
} | ||
} |
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.
nit: What do you think about adding a couple comments describing what the test is doing / what its goal is? I'm having a hard time following what's setup and what's validation.
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.
LGTM!
cb3e794
to
bd1eb76
Compare
Ticket
RM-323
Description
Fixes an issue where we didn't wait for all pod states to make a call about the state of a job.
Test Plan
Has an automated test.
Checklist
docs/release-notes/
.See Release Note for details.