-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[Client] Add ray.client().disconnect() #16021
Conversation
cc @wuisawesome |
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.
LGTM. should we make sure ray.util.disconnect() does not show up in the docs?
For Philipp's comment on returning a context class that we can call .disconnect() on, will that still support the case when the user forgot to assign the context class to a variable?
|
||
server.stop(0) | ||
subprocess.check_output("ray stop --force", shell=True) | ||
|
||
|
||
@pytest.mark.skip() |
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.
Hmm, should this really be skipped?
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.
OOps, that was a debugging thing (I was trying to figure out why a test-test interaction was causing problems).
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.
Thanks, this looks great (I made a few small notes for changes)
0529523
to
bddc3c2
Compare
@ijrsvt should we merge this? |
I'm rerunning the tests because Travis failed (and re-running did not work).
…On Wed, May 26, 2021 at 7:04 AM Ameer Haj Ali ***@***.***> wrote:
@ijrsvt <https://github.com/ijrsvt> should we merge this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16021 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFC5KQSXBVGTK4YY77ZNCCDTPT5WZANCNFSM45NNSERQ>
.
|
@ijrsvt is this mergable? |
FYI This passed Travis! |
Why are these changes needed?
ray.util.disconnect()
namespaces
(by addingimport ray
instead ofimport
).Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.