-
Notifications
You must be signed in to change notification settings - Fork 357
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: set max slots and checkpoint gc policy should comply with config policies #10140
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10140 +/- ##
==========================================
+ Coverage 54.71% 54.77% +0.06%
==========================================
Files 1266 1266
Lines 159970 159988 +18
Branches 3662 3662
==========================================
+ Hits 87525 87635 +110
+ Misses 72312 72220 -92
Partials 133 133
Flags with carried forward coverage won't be shown. Click here to find out more.
|
✅ Deploy Preview for determined-ui canceled.
|
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.
patch experiment really sketches me out. e.SetGroupWeight(*newResources.Weight)
and all call e.db.SaveExperimentConfig(e.ID, e.activeConfig)
and so does the endpoint itself. i feel like there are definitely calls where i could make the exp in memory state and db state inconsistent..?
hmm yea good point. |
2a432e3
to
b75303c
Compare
b75303c
to
78f0e8f
Compare
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.
I wanna note that technically, users can still override the name and description of an experiment with det e set name
and det e set description
, but I think that should be allowed, @kkunapuli wdyt?
Yeah, I agree. |
7693b6d
to
c287a09
Compare
c287a09
to
caa9179
Compare
Ticket
CM-590
Description
This PR fixes a couple of issues:
det e set max-slots
to comply with invariant config and constraintsdet e set gc-policy
to comply with invariant configsTest Plan
Checklist
docs/release-notes/
See Release Note for details.