chore: remove deprecated labels config option #9609
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 #9609 +/- ##
=======================================
Coverage 52.88% 52.88%
=======================================
Files 1255 1255
Lines 153086 153084 -2
Branches 3230 3230
=======================================
Hits 80964 80964
+ Misses 71971 71969 -2
Partials 151 151
Flags with carried forward coverage won't be shown. Click here to find out more.
|
kkunapuli
force-pushed
the
kunapuli/delete-deprecated-labels-agent-config
branch
from
b6578a6
to
22cfc6a
Compare
|
|
||
| - Cluster: ``resources.agent_label`` task option and agent config ``label`` option have been | ||
| removed. Beginning with 0.20.0 release, these options have been ignored. Please remove any | ||
| remaining references from configuration files. |
There was a problem hiding this comment.
can we add "Use resource_pool feature instead"?
| @@ -312,7 +312,10 @@ def main(det_callback, tb_callback, model_args, data_args, training_args): | |||
| id2label[str(i)] = label | |||
|
|
|||
| # Load the accuracy metric from the datasets package | |||
| metric = datasets.load_metric("accuracy", trust_remote_code=True,) | |||
| metric = datasets.load_metric( | |||
There was a problem hiding this comment.
that's an odd random reformat. maybe removing the last comma would've been okay as well?
There was a problem hiding this comment.
I can undo this; it came from auto-formatting.
| @@ -49,7 +49,7 @@ message Agent { | |||
| // A map of container id to all containers assigned to this agent. | |||
| map<string, determined.container.v1.Container> containers = 4; | |||
| // This field has been deprecated and will be empty. | |||
| string label = 5; | |||
| string label = 5 [deprecated = true]; | |||
There was a problem hiding this comment.
can we simply remove this and other protobuf field, instead of making them deprecated?
There was a problem hiding this comment.
If we do, make -C proto check fails.
for example,
buf lint
buf breaking --against buf.image.bin
src/determined/api/v1/agent.proto:12:1:Previously present field "5" with name "label" on message "GetAgentsRequest" was deleted.
There was a problem hiding this comment.
there's make gen-buf-image in there which'll force-regenerate the buf image. this is a protective measure against breaking an API accidentally but here we break it explicitly
There was a problem hiding this comment.
Harness changes look fine. +1 to @ioga's comment to mention resource_pool in the release note. Otherwise LGTM.
Ticket
RM-343
Description
Labelwas deprecated and removed from agent configuration over a year ago. Reduce confusion by removing references from the code and docs.If
labelis still present in an agent config, the following error message will be logged:Test Plan
None needed.
Checklist
docs/release-notes/See Release Note for details.