-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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 ]Fix bug for set gcs port environment variable #34367
[ Core ]Fix bug for set gcs port environment variable #34367
Conversation
48ed97e
to
21ba161
Compare
Signed-off-by: XiaodongLv <[email protected]>
21ba161
to
508efed
Compare
Signed-off-by: XiaodongLv <[email protected]>
Signed-off-by: XiaodongLv <[email protected]>
…env_GCS_PORT_ENVIRONMENT_VARIABLE_to_support_change_port_for_starting_ray
Signed-off-by: XiaodongLv <[email protected]>
python/ray/tests/BUILD
Outdated
@@ -304,6 +304,7 @@ py_test_module_list( | |||
"test_usage_stats.py", | |||
"test_placement_group_3.py", | |||
"test_placement_group_5.py", | |||
"test_env.py", |
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.
"test_env.py", | |
"test_set_gcs_server_port.py", |
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 have changed it.
Signed-off-by: XiaodongLv <[email protected]>
python/ray/_private/node.py
Outdated
@@ -270,7 +270,16 @@ def __init__( | |||
|
|||
# Pick a GCS server port. | |||
if head: | |||
gcs_server_port = os.getenv(ray_constants.GCS_PORT_ENVIRONMENT_VARIABLE) | |||
gcs_server_port_environment = os.getenv( |
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.
ray_constants.py has a helper function called env_integer()
, lets use that.
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.
Thankyou, I will change it.
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.
@jjyao I have completed it
Signed-off-by: XiaodongLv <[email protected]>
@@ -270,7 +270,11 @@ def __init__( | |||
|
|||
# Pick a GCS server port. | |||
if head: | |||
gcs_server_port = os.getenv(ray_constants.GCS_PORT_ENVIRONMENT_VARIABLE) | |||
from ray.autoscaler._private.constants import env_integer |
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.
can you use the method under ray._private.ray_constants, not the autoscaler package? (not sure why it has the same method there)
ray.shutdown() | ||
|
||
|
||
# If RAY_GCS_SERVER_PORT is set as string, it will be ignored so that |
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.
The comment and test doesn't seem to match?
Hmm it seems duplicate of #34482 (comment). cc @scv119 to confirm |
Duplicate to #34482 . closing |
Why are these changes needed?
When RAY_GCS_SERVER_PORT is set, ray.init() is executed locally will failed.
TypeError: '>' not supported between instances of 'str' and 'int'.
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.