[Autoscaler][v2] Fix instance_type_name in autoscaling state#62101
Conversation
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the autoscaling state by including provider-specific instance type names for pending and failed requests as well as pending instances. It updates the _fill_autoscaling_state function to perform these lookups via the autoscaling_config and adjusts the test suite to verify these new fields. Review feedback highlights a bug in the test aggregation logic that could lead to incorrect counts and suggests refactoring redundant lookups into a mapping for better efficiency and maintainability.
weimingdiit
force-pushed
the
fix/v2-autoscaling-state-instance-type-fields
branch
2 times, most recently
from
863ef7a to
b465a20
Compare
ray-gardener
Bot
added
core
weimingdiit
force-pushed
the
fix/v2-autoscaling-state-instance-type-fields
branch
from
b465a20 to
0ad4b4a
Compare
![cursor[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 0ad4b4a1c43032a96151a1233b908e82459f6ac7. Configure here.
|
Hi @rueian, the code has been updated. Could you please take another look? |
Populate `instance_type_name` in the autoscaling state generated by the v2 reconciler. Previously `_fill_autoscaling_state()` only populated `ray_node_type_name` for pending instance requests, pending instances, and failed instance requests. As a result, downstream consumers could observe empty provider instance types in parsed autoscaler status. Use `autoscaling_config.get_provider_instance_type()` to fill `instance_type_name`. Also harden the provider instance type lookup against missing node configs and update the reconciler unit test to assert both fields. Signed-off-by: weimingdiit <weimingdiit@gmail.com>
Signed-off-by: weimingdiit <weimingdiit@gmail.com>
weimingdiit
force-pushed
the
fix/v2-autoscaling-state-instance-type-fields
branch
from
3167c3a to
e7259eb
Compare
|
Hi @edoakes, please help merge this PR 🙏 |
1 similar comment
|
Hi @edoakes, please help merge this PR 🙏 |
|
@weimingdiit thanks for the contribution! |
…ject#62101) Populate `instance_type_name` in the autoscaling state generated by the v2 reconciler. Previously `_fill_autoscaling_state()` only populated `ray_node_type_name` for pending instance requests, pending instances, and failed instance requests. As a result, downstream consumers could observe empty provider instance types in parsed autoscaler status. Use `autoscaling_config.get_provider_instance_type()` to fill `instance_type_name`. ## Related issues ray-project#62100 --------- Signed-off-by: weimingdiit <weimingdiit@gmail.com>
3 participants
Description
Populate
instance_type_namein the autoscaling state generated by the v2 reconciler.Previously
_fill_autoscaling_state()only populatedray_node_type_namefor pending instance requests, pending instances, and failed instance requests. As a result, downstream consumers could observe empty provider instance types in parsed autoscaler status.Use
autoscaling_config.get_provider_instance_type()to fillinstance_type_name.Related issues
#62100