[core] Migrate wait_for_condition and async_wait_for_condition from _private to _common by abrarsheikh · Pull Request #53652 · ray-project/ray

abrarsheikh · 2025-06-09T03:13:15Z

…private to _common Signed-off-by: abrar <abrar@anyscale.com>

Signed-off-by: abrar <abrar@anyscale.com>

Copilot

Pull Request Overview

This PR migrates the wait_for_condition and async_wait_for_condition functions from the _private module to the _common module to centralize test utility functions. Key changes include:

Updating various test files to import wait_for_condition from ray._common.test_utils instead of ray._private.test_utils.
Removing the local implementations of wait_for_condition and async_wait_for_condition from ray/_private/test_utils.py.
Adding tests in ray/_common/tests to verify the new implementations.

Reviewed Changes

Copilot reviewed 237 out of 237 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
python/ray/dashboard/modules/event/tests/test_export_task.py	Changed import of wait_for_condition to common module.
python/ray/dashboard/modules/event/tests/test_event.py	Updated import to common module and removed duplicate wait_for_condition import.
python/ray/dag/tests/test_output_node.py	Updated import to common module for wait_for_condition.
python/ray/dag/tests/experimental/test_multi_node_dag.py	Reordered imports to use common wait_for_condition.
python/ray/dag/tests/experimental/test_mocked_nccl_dag.py	Updated import to common wait_for_condition.
python/ray/autoscaler/v2/tests/test_subscribers.py	Updated import of wait_for_condition to common module.
python/ray/autoscaler/v2/tests/test_sdk.py	Updated import of wait_for_condition to common module.
python/ray/autoscaler/v2/tests/test_node_provider.py	Reordered imports for wait_for_condition migration.
python/ray/autoscaler/v2/tests/test_e2e.py	Updated wait_for_condition import to common module.
python/ray/autoscaler/v2/tests/test_autoscaler.py	Updated wait_for_condition import to common module.
python/ray/air/tests/test_errors.py	Updated wait_for_condition import to common module.
python/ray/_private/test_utils.py	Removed wait_for_condition and async_wait_for_condition functionality.
python/ray/_private/state_api_test_utils.py	Updated import of test_utils to reference common module.
python/ray/_common/tests/test_wait_for_condition.py	Added tests for both wait_for_condition and async_wait_for_condition.
python/ray/_common/tests/test_signal_semaphore_utils.py	Updated wait_for_condition import to common module.
python/ray/_common/test_utils.py	Added new implementations for wait_for_condition and async_wait_for_condition.
doc/source/serve/doc_code/http_guide/disconnects.py	Updated wait_for_condition import to common module.
cpp/test_submit_cpp_job.py	Updated wait_for_condition import to common module.
ci/lint/pydoclint-baseline.txt	Updated baseline to reflect changes in wait_for_condition docstrings.

edoakes

Heroic 💪 thanks for the great test cases as well

release/nightly_tests/stress_tests/test_state_api_scale.py

Signed-off-by: abrar <abrar@anyscale.com>

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

aslonnie

this is a really big PR.. wonder if there are some ways to do the migration more incrementally.

also, some of the import orders look a bit off.

aslonnie · 2025-06-10T00:56:03Z

python/ray/_common/tests/test_signal_semaphore_utils.py

 import ray
 from ray._common.test_utils import SignalActor, Semaphore
-from ray._private.test_utils import wait_for_condition
+from ray._common.test_utils import wait_for_condition


these should be merged into a single line (if isort is enabled)?

aslonnie · 2025-06-10T00:56:51Z

python/ray/dag/tests/experimental/test_mocked_nccl_dag.py

 import os
 import sys
+
+from ray._common.test_utils import wait_for_condition


why is this imported before torch and pytest?

abrarsheikh · 2025-06-10T06:02:24Z

this is a really big PR.. wonder if there are some ways to do the migration more incrementally.

also, some of the import orders look a bit off.

We should take a separate AI to fix the isort logic.

My only issue with incremental rollouts is that it's not possible to restrict people from using the old callsite during migrations, hence I prefer to rip the band-aid off in one go.

follow up of #53652 Signed-off-by: abrar <abrar@anyscale.com>

…private to _common (#53652) Fixes #53478 --------- Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com> Co-authored-by: Edward Oakes <ed.nmi.oakes@gmail.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

follow up of #53652 Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

…private to _common (#53652) Fixes #53478 --------- Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com> Co-authored-by: Edward Oakes <ed.nmi.oakes@gmail.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

follow up of #53652 Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

…61363) ## Description This PR continues the migration from `ray._private` to `ray._common` so that Ray Serve (and other libraries) do not depend on internal private APIs. It migrates `tls_utils` and logging constants out of `_private` into `_common`, and updates all consumers (Serve, dashboard, client server) to import from the new locations. **Changes:** 1. **`ray._common.tls_utils`** - Moved `generate_self_signed_tls_certs()` from `_private.tls_utils` into `_common.tls_utils` (it only depended on `_common.network_utils`). - `_private.tls_utils` re-exports it for backward compatibility. - **Serve:** `serve/tests/test_https_proxy.py` now imports from `ray._common.tls_utils`. - **Other consumers:** `_private/grpc_utils.py`, `_private/test_utils.py`, `dashboard/agent.py`, `util/client/server/proxier.py`, `util/client/server/server.py` updated. 2. **`ray._common.logging_constants`** - New module containing `LOGRECORD_STANDARD_ATTRS` (frozenset), `LOGGER_FLATTEN_KEYS`, and `LogKey` enum — all moved from `_private.ray_logging.constants`. - `_private.ray_logging.constants` is deleted; `_private.ray_logging.logging_config` now imports from `_common`. - **Serve:** `serve/schema.py` now imports `LOGRECORD_STANDARD_ATTRS` from `ray._common.logging_constants`. 3. **`ray._common.filters` / `ray._common.formatters`** - Updated internal imports to use `ray._common.logging_constants` instead of `ray._private.ray_logging.constants`. **Tests:** - New `ray/_common/tests/test_logging_constants.py` — tests the full `LOGRECORD_STANDARD_ATTRS` frozenset, all `LogKey` enum members, and `LOGGER_FLATTEN_KEYS`. - New `ray/_common/tests/test_tls_utils.py` — tests `generate_self_signed_tls_certs`. - Lint: ruff check and format pass on all changed files. ## Related issues Fixes #53478 (partial: migrates Serve-side imports for `tls_utils` and logging constants; remaining items like `runtime_env_uri` and `worker_compat` can be done in follow-up PRs). ## Additional information - **Backward compatibility:** Existing code that imports `generate_self_signed_tls_certs` from `_private.tls_utils` continues to work via re-export. `_private.ray_logging.constants` is removed since all consumers have been updated. - **Scope:** Some Serve files still use `ray._private.worker.*` as attribute access (e.g. `global_worker`, `_global_node`) without a direct import; migrating those would require exposing them via `_common` and is left for a separate PR. - Sample PRs referenced in the issue: #53457 (signal/semaphore to _common), #53652 (wait_for_condition to _common). --------- Signed-off-by: mkdev11 <MkDev11@users.noreply.github.com> Co-authored-by: mkdev11 <MkDev11@users.noreply.github.com>

[core] Migrate wait_for_condition and async_wait_for_condition from _…

34ca7a0

…private to _common Signed-off-by: abrar <abrar@anyscale.com>

abrarsheikh added the go add ONLY when ready to merge, run all tests label Jun 9, 2025

abrarsheikh added 3 commits June 9, 2025 05:00

fix tests

7af2586

Signed-off-by: abrar <abrar@anyscale.com>

add to build file

9ae5309

Signed-off-by: abrar <abrar@anyscale.com>

generate baseline

165d71f

Signed-off-by: abrar <abrar@anyscale.com>

abrarsheikh marked this pull request as ready for review June 9, 2025 16:34

Copilot AI review requested due to automatic review settings June 9, 2025 16:34

abrarsheikh requested review from a team, GeneDer, akshay-anyscale, aslonnie, edoakes and zcin as code owners June 9, 2025 16:34

Copilot AI reviewed Jun 9, 2025

View reviewed changes

edoakes approved these changes Jun 9, 2025

View reviewed changes

release/nightly_tests/stress_tests/test_state_api_scale.py Outdated Show resolved Hide resolved

edoakes enabled auto-merge (squash) June 9, 2025 16:38

abrarsheikh added 2 commits June 9, 2025 16:40

Merge branch 'master' into 53478-abrar-wait

5a9d27b

fix test

31922d2

Signed-off-by: abrar <abrar@anyscale.com>

github-actions bot disabled auto-merge June 9, 2025 16:42

abrarsheikh and others added 2 commits June 9, 2025 20:08

Merge branch 'master' into 53478-abrar-wait

8534ca1

Merge branch 'master' into 53478-abrar-wait

33793c1

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

edoakes enabled auto-merge (squash) June 10, 2025 00:26

github-actions bot disabled auto-merge June 10, 2025 00:26

aslonnie reviewed Jun 10, 2025

View reviewed changes

aslonnie approved these changes Jun 10, 2025

View reviewed changes

edoakes merged commit 42fa5af into master Jun 10, 2025
6 checks passed

edoakes deleted the 53478-abrar-wait branch June 10, 2025 14:02

abrarsheikh mentioned this pull request Jun 10, 2025

fix CI, wrong import path #53715

Merged

aslonnie pushed a commit that referenced this pull request Jun 10, 2025

fix CI, wrong import path (#53715)

c0d7e84

follow up of #53652 Signed-off-by: abrar <abrar@anyscale.com>

elliot-barn pushed a commit that referenced this pull request Jun 18, 2025

fix CI, wrong import path (#53715)

5ce2c10

follow up of #53652 Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

elliot-barn pushed a commit that referenced this pull request Jul 2, 2025

fix CI, wrong import path (#53715)

b71daad

follow up of #53652 Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

MkDev11 mentioned this pull request Feb 26, 2026

[core|serve] Migrate serve imports from ray._private to ray._common #61363

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Migrate wait_for_condition and async_wait_for_condition from _private to _common #53652

[core] Migrate wait_for_condition and async_wait_for_condition from _private to _common #53652
edoakes merged 8 commits intomasterfrom
53478-abrar-wait

abrarsheikh commented Jun 9, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

edoakes left a comment

Uh oh!

Uh oh!

aslonnie left a comment

Uh oh!

aslonnie Jun 10, 2025

Uh oh!

aslonnie Jun 10, 2025

Uh oh!

abrarsheikh commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

abrarsheikh commented Jun 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

edoakes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aslonnie left a comment

Choose a reason for hiding this comment

Uh oh!

aslonnie Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

aslonnie Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

abrarsheikh commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants