[core] move the `RayletClientInterface` to its own build target by rueian · Pull Request #56101 · ray-project/ray

rueian · 2025-08-29T22:56:45Z

Why are these changes needed?

A new //src/ray/raylet_client:raylet_client_interface target containing only the RayletClientInterface.
A new //src/ray/raylet_client:raylet_client_pool target moved from the node_manager.
A new //src/ray/raylet_client:node_manager_client target moved from the node_manager.
Remove using statements in the raylet_client.h that allow others to omit ray:: implicitly.

There are no behavioral changes.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Rueian <rueian@anyscale.com>

codope · 2025-09-01T04:10:21Z

src/ray/raylet_client/BUILD.bazel

-exports_files([
-    "raylet_client.h",
-])
+ray_cc_library(


What’s the visibility on the new targets? De we need all the targets to be public?

Most of them are intended to be public indeed, except for the "node_manager_client", which should be :__subpackages__.

please add explicit visibility tags for them (even public ones)

at some point I want to remove our default public visibility from ray.bzl

src/ray/raylet_client/raylet_client.h

codope · 2025-09-01T04:28:53Z

src/ray/raylet_client/node_manager_client.h

Why not eliminate the RayletClient wrapper as well?

ray/src/ray/rpc/node_manager/node_manager_client.h

Lines 34 to 46 in 62b6bb4

namespace raylet {

class RayletClient;

}

namespace rpc {

/// TODO(dayshah): https://github.com/ray-project/ray/issues/54816 Kill this completely.

/// This class is only used by the RayletClient which is just a wrapper around this. This

/// exists for the legacy reason that all the function definitions in RayletClient have to

/// change if you move the things in here into RayletClient.

class NodeManagerClient {

public:

friend class raylet::RayletClient;

Can I do that in a follow-up? It won't be too hard, I guess, but this PR is already quite big...

Yesh, agree let's do in a followup.

Signed-off-by: Rueian <rueian@anyscale.com>

edoakes

Huge improvement that has been on my TODO list -- thank you!

edoakes · 2025-09-02T20:03:26Z

src/ray/rpc/BUILD.bazel

-ray_cc_library(
-    name = "node_manager_client",
-    srcs = ["node_manager/raylet_client_pool.cc"],
-    hdrs = [
-        "node_manager/node_manager_client.h",
-        "node_manager/raylet_client_pool.h",
-    ] + [
-        # TODO(eoakes): these are needed due to a circular dependency:
-        # raylet_client_pool.cc -> raylet_client.h -> node_manager_client.h
-        "//src/ray/raylet_client:raylet_client.h",
-    ],
-    visibility = ["//visibility:public"],
-    deps = [
-        ":client_call",
-        ":grpc_client",
-        "//src/ray/common:id",
-        "//src/ray/gcs/gcs_client:gcs_client_lib",
-        "//src/ray/protobuf:node_manager_cc_grpc",
-        "//src/ray/util:network_util",
-    ] + [
-        # TODO(eoakes): these three come from raylet_client.h, remove after breaking the circular dependency.
-        "//src/ray/ipc:client_connection",
-        "//src/ray/common:ray_object",
-        "//src/ray/common:task_common",
-    ],
-)
-


edoakes · 2025-09-02T20:04:13Z

src/ray/raylet_client/BUILD.bazel

-exports_files([
-    "raylet_client.h",
-])
+ray_cc_library(


please add explicit visibility tags for them (even public ones)

at some point I want to remove our default public visibility from ray.bzl

edoakes · 2025-09-02T20:04:56Z

Please add visibility in follow up

…project#56101) ## Why are these changes needed? 1. A new `//src/ray/raylet_client:raylet_client_interface` target containing only the `RayletClientInterface`. 2. A new `//src/ray/raylet_client:raylet_client_pool` target moved from the node_manager. 3. A new `//src/ray/raylet_client:node_manager_client` target moved from the node_manager. 4. Remove `using` statements in the `raylet_client.h` that allow others to omit `ray::` implicitly. There are no behavioral changes. ## Related issue number  ## Checks - [x] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [x] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Rueian <rueian@anyscale.com> Signed-off-by: sampan <sampan@anyscale.com>

…project#56101) ## Why are these changes needed? 1. A new `//src/ray/raylet_client:raylet_client_interface` target containing only the `RayletClientInterface`. 2. A new `//src/ray/raylet_client:raylet_client_pool` target moved from the node_manager. 3. A new `//src/ray/raylet_client:node_manager_client` target moved from the node_manager. 4. Remove `using` statements in the `raylet_client.h` that allow others to omit `ray::` implicitly. There are no behavioral changes. ## Related issue number  ## Checks - [x] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [x] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Rueian <rueian@anyscale.com> Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>

…project#56101) ## Why are these changes needed? 1. A new `//src/ray/raylet_client:raylet_client_interface` target containing only the `RayletClientInterface`. 2. A new `//src/ray/raylet_client:raylet_client_pool` target moved from the node_manager. 3. A new `//src/ray/raylet_client:node_manager_client` target moved from the node_manager. 4. Remove `using` statements in the `raylet_client.h` that allow others to omit `ray::` implicitly. There are no behavioral changes. ## Related issue number  ## Checks - [x] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [x] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Rueian <rueian@anyscale.com> Signed-off-by: yenhong.wong <yenhong.wong@grabtaxi.com>

## Why are these changes needed? 1. A new `//src/ray/raylet_client:raylet_client_interface` target containing only the `RayletClientInterface`. 2. A new `//src/ray/raylet_client:raylet_client_pool` target moved from the node_manager. 3. A new `//src/ray/raylet_client:node_manager_client` target moved from the node_manager. 4. Remove `using` statements in the `raylet_client.h` that allow others to omit `ray::` implicitly. There are no behavioral changes. ## Related issue number  ## Checks - [x] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [x] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Rueian <rueian@anyscale.com> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>

…project#56101) ## Why are these changes needed? 1. A new `//src/ray/raylet_client:raylet_client_interface` target containing only the `RayletClientInterface`. 2. A new `//src/ray/raylet_client:raylet_client_pool` target moved from the node_manager. 3. A new `//src/ray/raylet_client:node_manager_client` target moved from the node_manager. 4. Remove `using` statements in the `raylet_client.h` that allow others to omit `ray::` implicitly. There are no behavioral changes. ## Related issue number  ## Checks - [x] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [x] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Rueian <rueian@anyscale.com>

rueian force-pushed the raylet-client-interface branch 2 times, most recently from 04b8f7e to c0debbd Compare August 29, 2025 23:24

rueian added the go add ONLY when ready to merge, run all tests label Aug 30, 2025

[core] move the RayletClientInterface to its own build target

c96c7df

Signed-off-by: Rueian <rueian@anyscale.com>

rueian force-pushed the raylet-client-interface branch from c0debbd to c96c7df Compare August 30, 2025 05:07

Merge branch 'master' into raylet-client-interface

7f2cad1

Signed-off-by: Rueian <rueian@anyscale.com>

rueian force-pushed the raylet-client-interface branch from 4671ebe to 7f2cad1 Compare August 30, 2025 16:06

rueian marked this pull request as ready for review August 30, 2025 18:21

rueian requested a review from a team as a code owner August 30, 2025 18:21

ray-gardener bot added the core Issues that should be addressed in Ray Core label Aug 30, 2025

codope reviewed Sep 1, 2025

View reviewed changes

rueian added 2 commits September 1, 2025 14:18

Merge branch 'master' into raylet-client-interface

62d5740

Signed-off-by: Rueian <rueian@anyscale.com>

visibility

1dd6013

Signed-off-by: Rueian <rueian@anyscale.com>

edoakes approved these changes Sep 2, 2025

View reviewed changes

edoakes merged commit 1d2c95f into ray-project:master Sep 2, 2025
5 checks passed

rueian mentioned this pull request Sep 5, 2025

[core] replace node_manager_client with raylet_client_lib #56261

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] move the `RayletClientInterface` to its own build target#56101

[core] move the `RayletClientInterface` to its own build target#56101
edoakes merged 4 commits intoray-project:masterfrom
rueian:raylet-client-interface

rueian commented Aug 29, 2025 •

edited

Loading

Uh oh!

codope Sep 1, 2025

Uh oh!

rueian Sep 1, 2025 •

edited

Loading

Uh oh!

edoakes Sep 2, 2025

Uh oh!

Uh oh!

codope Sep 1, 2025

Uh oh!

rueian Sep 1, 2025

Uh oh!

codope Sep 3, 2025

Uh oh!

edoakes left a comment

Uh oh!

edoakes Sep 2, 2025

Uh oh!

edoakes Sep 2, 2025

Uh oh!

edoakes commented Sep 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	namespace raylet {
	class RayletClient;
	}

	namespace rpc {

	/// TODO(dayshah): https://github.com/ray-project/ray/issues/54816 Kill this completely.
	/// This class is only used by the RayletClient which is just a wrapper around this. This
	/// exists for the legacy reason that all the function definitions in RayletClient have to
	/// change if you move the things in here into RayletClient.
	class NodeManagerClient {
	public:
	friend class raylet::RayletClient;

Conversation

rueian commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

codope Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

rueian Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edoakes Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codope Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

rueian Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

codope Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

edoakes left a comment

Choose a reason for hiding this comment

Uh oh!

edoakes Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

edoakes Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

edoakes commented Sep 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rueian commented Aug 29, 2025 •

edited

Loading

rueian Sep 1, 2025 •

edited

Loading