-
Notifications
You must be signed in to change notification settings - Fork 354
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: job queue test failures #8843
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8843 +/- ##
==========================================
- Coverage 47.58% 47.57% -0.01%
==========================================
Files 1068 1068
Lines 170241 170241
Branches 2240 2240
==========================================
- Hits 81009 80999 -10
- Misses 89074 89084 +10
Partials 158 158
Flags with carried forward coverage won't be shown. Click here to find out more. |
master/pkg/tasks/task_generic.go
Outdated
@@ -99,7 +99,7 @@ func (s GenericTaskSpec) SetJobPriority(priority int) error { return nil } | |||
|
|||
// SetWeight todo. | |||
func (s GenericTaskSpec) SetWeight(weight float64) error { | |||
s.Base.ResourcesConfig.SetWeight(weight) | |||
s.GenericTaskConfig.Resources.SetWeight(weight) |
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.
will this actually affect priority in scheduling?
I see commands set the group weight in the resource manager
determined/master/internal/command/command_job_service.go
Lines 62 to 72 in ba03375
switch err := c.rm.SetGroupWeight(sproto.SetGroupWeight{ | |
Weight: weight, | |
ResourcePool: c.Config.Resources.ResourcePool, | |
JobID: c.jobID, | |
}).(type) { | |
case nil: | |
case rmerrors.UnsupportedError: | |
c.syslog.WithError(err).Debug("ignoring unsupported call to set group weight") | |
default: | |
return fmt.Errorf("setting group weight for command: %w", err) | |
} |
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.
The implementation of toV1Job
and SetWeight
was originally to just allow det job list
to work when a generic task was running. We will still need to fully implement all the functions later. If you see the priority and resource pool functions are still unimplemented
4e39b62
to
1dfb520
Compare
* hotfix for test * ensure task completion * remove comment * update test helpers * default timeout (cherry picked from commit 08dfa43)
* hotfix for test * ensure task completion * remove comment * update test helpers * default timeout
Description
Fixing generic task tests that cause the job queue test to fail
Test Plan
e2e tests should pass, specifically
test_job_queue.py
&test_generic_tasks.py
Checklist
docs/release-notes/
.See Release Note for details.
Ticket