-
Notifications
You must be signed in to change notification settings - Fork 7k
[Core]Separate Environment Variables for ray.init() and ray ctl to Reflect Different Protocols and Ports #55189
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
[Core]Separate Environment Variables for ray.init() and ray ctl to Reflect Different Protocols and Ports #55189
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @machichima, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've addressed a potential source of confusion in Ray's environment variable usage. Previously, both ray.init() and ray ctl relied on the single RAY_ADDRESS environment variable, despite connecting to different services (GCS gRPC for ray.init() and Dashboard HTTP for ray ctl) with different protocols and default ports. To resolve this, I've introduced RAY_API_SERVER_ADDRESS specifically for ray ctl connections, allowing RAY_ADDRESS to remain dedicated to ray.init(). This change clarifies the intended use of each environment variable, making it easier for users to configure their Ray cluster connections correctly without conflicts.
Highlights
- New Environment Variable: I've introduced a new environment variable,
RAY_API_SERVER_ADDRESS, to specifically designate the address for connecting to the Ray API server, which is primarily used byray ctl. - CLI Updates for
ray ctl: I've updated the command-line interface forray ctlto inform users about the newRAY_API_SERVER_ADDRESSand ensure that it is prioritized for connection addresses, while still maintaining backward compatibility by falling back toRAY_ADDRESSif the new variable is not set. - Internal Logic and Documentation Alignment: I've modified the internal logic and documentation within the Ray dashboard utilities and SDK to reflect this new precedence, ensuring that
ray.init()continues to correctly useRAY_ADDRESSfor GCS connections, whileray ctlleverages the new variable for the dashboard API.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
c131ad0 to
1270db9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new environment variable, RAY_API_SERVER_ADDRESS, for ray ctl commands to distinguish it from RAY_ADDRESS used by ray.init(). This change effectively resolves potential confusion between the different protocols and ports used by these tools. The implementation correctly includes fallback logic to RAY_ADDRESS for backward compatibility, and all related documentation and help messages have been updated accordingly. I have one suggestion to improve code quality by avoiding redundant lookups of environment variables.
python/ray/dashboard/utils.py
Outdated
| if os.environ.get("RAY_API_SERVER_ADDRESS"): | ||
| logger.debug(f"Using RAY_API_SERVER_ADDRESS={os.environ['RAY_API_SERVER_ADDRESS']}") | ||
| address = os.environ["RAY_API_SERVER_ADDRESS"] | ||
| # Fall back to RAY_ADDRESS if RAY_API_SERVER_ADDRESS not set | ||
| elif os.environ.get("RAY_ADDRESS"): | ||
| logger.debug(f"Using RAY_ADDRESS={os.environ['RAY_ADDRESS']}") | ||
| address = os.environ["RAY_ADDRESS"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve performance and readability, you can use the walrus operator := (available in Python 3.8+) to avoid multiple lookups for the same environment variable.
| if os.environ.get("RAY_API_SERVER_ADDRESS"): | |
| logger.debug(f"Using RAY_API_SERVER_ADDRESS={os.environ['RAY_API_SERVER_ADDRESS']}") | |
| address = os.environ["RAY_API_SERVER_ADDRESS"] | |
| # Fall back to RAY_ADDRESS if RAY_API_SERVER_ADDRESS not set | |
| elif os.environ.get("RAY_ADDRESS"): | |
| logger.debug(f"Using RAY_ADDRESS={os.environ['RAY_ADDRESS']}") | |
| address = os.environ["RAY_ADDRESS"] | |
| if api_server_address := os.environ.get("RAY_API_SERVER_ADDRESS"): | |
| logger.debug(f"Using RAY_API_SERVER_ADDRESS={api_server_address}") | |
| address = api_server_address | |
| # Fall back to RAY_ADDRESS if RAY_API_SERVER_ADDRESS not set | |
| elif ray_address := os.environ.get("RAY_ADDRESS"): | |
| logger.debug(f"Using RAY_ADDRESS={ray_address}") | |
| address = ray_address |
159fb66 to
27f1b63
Compare
| # Fall back to RAY_ADDRESS if RAY_API_SERVER_ADDRESS not set | ||
| elif ray_address := os.environ.get(ray_constants.RAY_ADDRESS_ENVIRONMENT_VARIABLE): | ||
| address = ray_address | ||
| logger.debug(f"Using RAY_ADDRESS={address}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need a test at class TestRayAddress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem! Added test in 9e8764c
ef026f2 to
5d656e9
Compare
Thanks for point these out! Updated in 5d656e9 |
owenowenisme
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
edoakes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall
| ray_api_server_address: str, | ||
| should_fail: bool, | ||
| ): | ||
| # Always use problematic client address to ensure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is "problematic client address"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means the wrong RAY_ADDRESS format (which we name RAY_ADDRESS as ray_client_address)
If the job cli use this address, it will surely fail. I use this to ensure job cli is dominated by RAY_API_SERVER_ADDRESS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. maybe we can make this more clear in the comment, like:
# Set a `RAY_ADDRESS` that would not work with the `ray job submit` CLI because it uses the `ray://` prefix.
# This verifies that the `RAY_API_SERVER_ADDRESS` env var takes precedence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Updated the comment in 79a50d0
| assert "hello" in stdout | ||
| assert "succeeded" in stdout | ||
|
|
||
| @pytest.mark.parametrize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are other existing PRs that use RAY_ADDRESS, could you please make sure to update them as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Do you mean opening some follow-up PR for other existing ones to change RAY_ADDRESS to RAY_API_SERVER_ADDRESS if related to ray job cli?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that's right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem!
-e Signed-off-by: machichima <[email protected]>
-e Signed-off-by: machichima <[email protected]>
-e Signed-off-by: machichima <[email protected]>
-e Signed-off-by: machichima <[email protected]>
-e Signed-off-by: machichima <[email protected]>
-e Signed-off-by: machichima <[email protected]>
1271402 to
b7cb367
Compare
|
Started CI tests & one minor nit, please ping me when tests pass! |
-e Signed-off-by: machichima <[email protected]>
|
Thanks @machichima! |
…flect Different Protocols and Ports (#55189) Currently, both `ray.init()` and ray ctl rely on the same environment variable RAY_ADDRESS to determine the default address to connect to a Ray cluster. However, in practice: `ray.init()` uses the `ray:// protocol` and connects via the GCS gRPC port (default `10001`) ray ctl uses the HTTP protocol (`http://`) and connects via the Dashboard port (default `8265`) This leads to potential confusion when setting the `RAY_ADDRESS` environment variable, as it may not be valid for both tools simultaneously. For example, setting `RAY_ADDRESS=http://localhost:8265/` would work for ray ctl, but not for `ray.init()`, which expects a `ray://` URI with the GCS port. In this PR, we do: - Keep `RAY_ADDRESS` for `ray.init()` - Use `RAY_API_SERVER_ADDRESS` for ray job ctl - Update docs: https://anyscale-ray--55189.com.readthedocs.build/en/55189/cluster/running-applications/job-submission/quickstart.html Closes #53226 Signed-off-by: sampan <[email protected]>
…flect Different Protocols and Ports (ray-project#55189) Currently, both `ray.init()` and ray ctl rely on the same environment variable RAY_ADDRESS to determine the default address to connect to a Ray cluster. However, in practice: `ray.init()` uses the `ray:// protocol` and connects via the GCS gRPC port (default `10001`) ray ctl uses the HTTP protocol (`http://`) and connects via the Dashboard port (default `8265`) This leads to potential confusion when setting the `RAY_ADDRESS` environment variable, as it may not be valid for both tools simultaneously. For example, setting `RAY_ADDRESS=http://localhost:8265/` would work for ray ctl, but not for `ray.init()`, which expects a `ray://` URI with the GCS port. In this PR, we do: - Keep `RAY_ADDRESS` for `ray.init()` - Use `RAY_API_SERVER_ADDRESS` for ray job ctl - Update docs: https://anyscale-ray--55189.com.readthedocs.build/en/55189/cluster/running-applications/job-submission/quickstart.html Closes ray-project#53226 Signed-off-by: Andrew Grosser <[email protected]>
…flect Different Protocols and Ports (ray-project#55189) Currently, both `ray.init()` and ray ctl rely on the same environment variable RAY_ADDRESS to determine the default address to connect to a Ray cluster. However, in practice: `ray.init()` uses the `ray:// protocol` and connects via the GCS gRPC port (default `10001`) ray ctl uses the HTTP protocol (`http://`) and connects via the Dashboard port (default `8265`) This leads to potential confusion when setting the `RAY_ADDRESS` environment variable, as it may not be valid for both tools simultaneously. For example, setting `RAY_ADDRESS=http://localhost:8265/` would work for ray ctl, but not for `ray.init()`, which expects a `ray://` URI with the GCS port. In this PR, we do: - Keep `RAY_ADDRESS` for `ray.init()` - Use `RAY_API_SERVER_ADDRESS` for ray job ctl - Update docs: https://anyscale-ray--55189.com.readthedocs.build/en/55189/cluster/running-applications/job-submission/quickstart.html Closes ray-project#53226 Signed-off-by: jugalshah291 <[email protected]>
…flect Different Protocols and Ports (#55189) Currently, both `ray.init()` and ray ctl rely on the same environment variable RAY_ADDRESS to determine the default address to connect to a Ray cluster. However, in practice: `ray.init()` uses the `ray:// protocol` and connects via the GCS gRPC port (default `10001`) ray ctl uses the HTTP protocol (`http://`) and connects via the Dashboard port (default `8265`) This leads to potential confusion when setting the `RAY_ADDRESS` environment variable, as it may not be valid for both tools simultaneously. For example, setting `RAY_ADDRESS=http://localhost:8265/` would work for ray ctl, but not for `ray.init()`, which expects a `ray://` URI with the GCS port. In this PR, we do: - Keep `RAY_ADDRESS` for `ray.init()` - Use `RAY_API_SERVER_ADDRESS` for ray job ctl - Update docs: https://anyscale-ray--55189.com.readthedocs.build/en/55189/cluster/running-applications/job-submission/quickstart.html Closes #53226 Signed-off-by: Douglas Strodtman <[email protected]>


Why are these changes needed?
Currently, both
ray.init()and ray ctl rely on the same environment variable RAY_ADDRESS to determine the default address to connect to a Ray cluster. However, in practice:ray.init()uses theray:// protocoland connects via the GCS gRPC port (default10001)ray ctl uses the HTTP protocol (
http://) and connects via the Dashboard port (default8265)This leads to potential confusion when setting the
RAY_ADDRESSenvironment variable, as it may not be valid for both tools simultaneously. For example, settingRAY_ADDRESS=http://localhost:8265/would work for ray ctl, but not forray.init(), which expects aray://URI with the GCS port.In this PR, we do:
RAY_ADDRESSforray.init()RAY_API_SERVER_ADDRESSfor ray job ctlResult Screenshot:
Related issue number
Closes #53226
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.