[Data] Remove constructor from ClusterAutoscaler base class#59733
Merged
bveeramani merged 2 commits intoDec 28, 2025
Merged
Conversation
Signed-off-by: machichima <nary12321@gmail.com>
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request successfully removes the constructor from the ClusterAutoscaler base class, simplifying its subclasses. The changes in DefaultClusterAutoscaler and DefaultClusterAutoscalerV2 correctly adapt to this by removing the super().__init__ call. The corresponding test files are also updated accordingly. I've found one minor opportunity for further cleanup in DefaultClusterAutoscaler to remove an unused dependency, which aligns well with the goal of this PR.
Comment on lines
+33
to
+35
| self._topology = topology | ||
| self._resource_manager = resource_manager | ||
| self._execution_id = execution_id |
Contributor
There was a problem hiding this comment.
The self._resource_manager is assigned but never used within the DefaultClusterAutoscaler class. It can be removed to simplify the code, which aligns with the goal of this PR. To fully remove the unused dependency, you would also need to remove resource_manager from the __init__ signature and from the call site in python/ray/data/_internal/cluster_autoscaler/__init__.py.
Suggested change
| self._topology = topology | |
| self._resource_manager = resource_manager | |
| self._execution_id = execution_id | |
| self._topology = topology | |
| self._execution_id = execution_id |
Contributor
Author
|
@bveeramani PTAL. Thank you! |
Signed-off-by: machichima <nary12321@gmail.com>
machichima
force-pushed
the
59684-remove-cluster-autoscaler-constructor
branch
from
575bb9a to
abae532
Compare
AYou0207
pushed a commit
to AYou0207/ray
that referenced
this pull request
Jan 13, 2026
…ect#59733) ## Description The constructor in ClusterAutoscaler base class is not necessary, and it adds complexity because it requires all ClusterAutoscaling implementations to accept the same dependencies This PR remove the constructor in ClusterAutoscaler. Sub-classes can prevent using the dependencies that are not used ## Related issues Closes ray-project#59684 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: machichima <nary12321@gmail.com> Signed-off-by: jasonwrwang <jasonwrwang@tencent.com>
zzchun
pushed a commit
to antgroup/ant-ray
that referenced
this pull request
Jan 29, 2026
…ect#59733) ## Description The constructor in ClusterAutoscaler base class is not necessary, and it adds complexity because it requires all ClusterAutoscaling implementations to accept the same dependencies This PR remove the constructor in ClusterAutoscaler. Sub-classes can prevent using the dependencies that are not used ## Related issues Closes ray-project#59684 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: machichima <nary12321@gmail.com>
zzchun
pushed a commit
to antgroup/ant-ray
that referenced
this pull request
Feb 5, 2026
…ect#59733) ## Description The constructor in ClusterAutoscaler base class is not necessary, and it adds complexity because it requires all ClusterAutoscaling implementations to accept the same dependencies This PR remove the constructor in ClusterAutoscaler. Sub-classes can prevent using the dependencies that are not used ## Related issues Closes ray-project#59684 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: machichima <nary12321@gmail.com>
zzchun
added a commit
to antgroup/ant-ray
that referenced
this pull request
Apr 15, 2026
aa640334e16 # Add util package c535e50e365 # [Data] - Add support for callable classes for UDFExpr (ray-project#56725) 0266765ec39 # [Data] Remove constructor from ClusterAutoscaler base class (ray-project#59733) 03416b2e69f # [Data] DefaultAutoscalerV2 doesn't scale nodes from zero (ray-project#59896) 207295e23db # [Data] Introduce seams to DefaultAutoscaler2 to make it more testable (ray-project#59933) c5eceb4ce7a # [Data] DefaultClusterAutoscalerV2 raises KeyError: 'CPU' Fix (ray-project#60208) 0bb15c93e5c # [data] Allow configuring DefaultClusterAutoscalerV2 thresholds via en… (ray-project#60133) e52695c23df # [Data] Fix autoscaler to respect user-configured resource limits (ray-project#60283) 15c929bbc21 # [Data] Reorganize sections of `DefaultAutoscalerV2` constructor (ray-project#60347) b5ffc068613 # [Data] Fix autoscaler to request previous allocation instead of empty resources when not scaling up (ray-project#60321)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The constructor in ClusterAutoscaler base class is not necessary, and it adds complexity because it requires all ClusterAutoscaling implementations to accept the same dependencies
This PR remove the constructor in ClusterAutoscaler. Sub-classes can prevent using the dependencies that are not used
Related issues
Closes #59684
Additional information