[core][autoscaler][v2] Fix invalid default stats factory in ClusterStatus#62934
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the ClusterStatus schema to initialize the stats field with a default gcs_request_time_s value of 0.0 and adds a unit test to verify these defaults. Feedback was provided to improve PEP 8 compliance by ensuring top-level functions in the test file are separated by two blank lines.
b9a7147 to
21e9bd3
Compare
21e9bd3 to
912e825
Compare
…atus Signed-off-by: weimingdiit <weimingdiit@gmail.com>
Signed-off-by: weimingdiit <weimingdiit@gmail.com>
Head branch was pushed to by a user without write access
912e825 to
53ff58d
Compare
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
|
Hi @edoakes, the previous auto-merge was disabled. Please merge this when you get a chance. Thanks! |
Signed-off-by: Rueian <rueiancsie@gmail.com>
Description
ClusterStatus.statscurrently usesfield(default_factory=Stats), butStatsrequires the positional argumentgcs_request_time_s.As a result,
ClusterStatus()cannot be default-constructed and raises:This PR fixes the invalid default by making ClusterStatus construct a valid default Stats object, and adds a regression test to cover the empty/default construction path.
This is a small schema-level fix and does not change the normal populated paths where callers already pass an explicit Stats(...) instance.
Related issues
#62933
Additional information
Implementation notes:
update ClusterStatus.stats to use a valid default Stats value add a regression test for ClusterStatus()