Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat][Core/Dashboard] Remove dashboard gRPC server #51956

Merged

Conversation

MortalHappiness
Copy link
Member

@MortalHappiness MortalHappiness commented Apr 3, 2025

Why are these changes needed?

After #51555, the dashboard gRPC server is not used by any modules. This PR removes it.

The CLI flag --dashboard-grpc-port is kept but deprecated.

Related issue number

N/A

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 :(

@MortalHappiness MortalHappiness added the go add ONLY when ready to merge, run all tests label Apr 3, 2025
@MortalHappiness MortalHappiness force-pushed the feature/remove-dashboard-grpc-server branch from 4b6a372 to 8efce81 Compare April 3, 2025 14:15
Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
@MortalHappiness MortalHappiness force-pushed the feature/remove-dashboard-grpc-server branch from 8efce81 to f102661 Compare April 3, 2025 18:12
@MortalHappiness MortalHappiness marked this pull request as ready for review April 4, 2025 16:47
@@ -526,7 +526,6 @@ def print_after(_obj):

def test_log_redirect_to_stderr(shutdown_only):
log_components = {
ray_constants.PROCESS_TYPE_DASHBOARD: "Dashboard head grpc address",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep this but check a different log message

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use "Starting dashboard metrics server on port" instead.

@jjyao
Copy link
Collaborator

jjyao commented Apr 6, 2025

cc @dayshah for the doc review.

@@ -134,7 +134,7 @@ In addition to ports specified above, the head node needs to open several more p
- ``--port``: Port of Ray (GCS server). The head node will start a GCS server listening on this port. Default: 6379.
- ``--ray-client-server-port``: Listening port for Ray Client Server. Default: 10001.
- ``--redis-shard-ports``: Comma-separated list of ports for non-primary Redis shards. Default: Random values.
- ``--dashboard-grpc-port``: The gRPC port used by the dashboard. Default: Random value.
- ``--dashboard-grpc-port``: (Deprecated) No longer used. Only kept for backward compatibility.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when would we fully remove?

@jjyao jjyao merged commit c402550 into ray-project:master Apr 6, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants