[autoscaler] Fix ray.autoscaler.sdk import issue #21795
Conversation
| @@ -26,6 +26,14 @@ def env_bool(key, default): | |||
| return default | |||
|
|
|||
|
|
|||
| # Whether event logging to driver is enabled. Set to 0 to disable. | |||
| AUTOSCALER_EVENTS = env_integer("RAY_SCHEDULER_EVENTS", 1) | |||
There was a problem hiding this comment.
This fixed the giant circular dependency.
| @@ -79,7 +73,9 @@ def validate_config(config: Dict[str, Any]) -> None: | |||
| if not isinstance(config, dict): | |||
| raise ValueError("Config {} is not a dictionary".format(config)) | |||
|
|
|||
| with open(RAY_SCHEMA_PATH) as f: | |||
| schema_path = os.path.join( | |||
There was a problem hiding this comment.
This fixed a small circular dependency.
| @@ -52,7 +52,7 @@ | |||
| from ray.worker import global_worker # type: ignore | |||
| from ray.util.debug import log_once | |||
|
|
|||
| import ray.autoscaler._private.subprocess_output_util as cmd_output_util | |||
There was a problem hiding this comment.
this fixed a minor circular dependency.
|
@AmeerHajAli this was the issue you thought was "trivial"? |
Perhaps. |
|
@DmitriGekhtman @ckw017 do either of you happen to know how to fix this lint error? It seems to want to lint a non-existent file :(. |
hah, that's from my abortive start at adding type checking to the autoscaler code, circa October 2020 |
wuisawesome
requested review from
edoakes,
ericl and
richardliaw
as code owners
wuisawesome
added
the
tests-ok
|
Rllib failure looks unrelated and broken in master. Merging. |
Why are these changes needed?
This PR moves the sdk to its own folder, then includes everything in
import ray.autoscaler.sdkin ray's import path.Note: that there were circular dependencies in naively doing this because the ray core now uses constants that were defined in the autoscaler for internal kv operations (and the autoscaler similarly calls into the ray core). The solution was to move those internal kv keys into ray core constants so the imports flow (more) one way.
Related issue number
closes #19840
Checks
scripts/format.shto lint the changes in this PR.