[client][docs] update docs for new client support in init #17333

ckw017 · 2021-07-26T04:51:45Z

Why are these changes needed?

Add docs for starting a client connection with ray.init, and details for using the ClientBuilder API for ray versions < 1.5.

Related issue number

Checks

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

…rnings

ckw017 · 2021-07-26T06:07:12Z

python/ray/worker.py

@@ -710,9 +720,9 @@ def init(
            and the API is subject to change.

    Returns:
-        If the provided address included a protocol (e.g. "ray://1.2.3.4")


Parentheses were throwing off sphinx doc generation (mixed up part inside the parens with return type)

nice catch here!

doc/source/cluster/ray-client.rst

AmeerHajAli

I like the new docs, thanks Chris! cc @richardliaw
I am not sure if we want to keep the API of pre 1.5, what @pcmoritz / @ijrsvt think?

… behavior when address passed

python/ray/worker.py

ijrsvt

This looks good to me! Just a few comments!

ckw017 · 2021-07-28T17:23:35Z

@richardliaw Drops the old docs and fixes the .connect call discussed in #16946

AmeerHajAli · 2021-07-29T14:10:48Z

doc/source/cluster/ray-client.rst


-You can also connect using the ClientBuilder API, but this will eventually be deprecated.
+**Local mode** is used when the address is prefixed with ``local://``. If no address is provided, i.e. ``ray.init("local://")``), a local cluster is created and connected to directly by attaching a driver. If an address is provided, i.e. ``ray.init("local://localhost:6379")``, Ray will attempt to attach a driver to an existing cluster at the specified address. Local mode accepts all of the same keywords as ``ray.init`` and uses them to configure the local cluster.


what is the difference between ray.init("local://localhost: 6379") over ray.init("localhost: 6379")
Also, in general "Local mode" is not clear (mainly it is not clear what local means).

Also, are you sure it connects directly and not over the ray client code path?

After syncing with Ian, it looks like this probably isn't even worth mentioning in the client docs (it doesn't even use the ray client, and there's not really a reason to use it over just ray.init()). Looks like it was more of a convenience thing for ray.client() so that you could just change the argument to ray.client().connect() instead of changing the entire thing to ray.init().

Should we then remove the special handling of ‘local://‘?
Also, cc @pcmoritz to double verify.

It'll be useful to keep the code for now, I think(?) the long term plan is to require any address passed into ray.init to start with something://, and ray.init("local://address") will just be used if to replicate current behavior of ray.init("address")

Thanks @ckw017 for following up on this. I think it is better to remove code and then later add it if we need it (we have the git diff/commits in case we want to retrieve it).

+1 to not support this for now, now that we have unified the ray.init API and the ClientBuilder API I don't think this functionality is that critical.

The idea here was to only support ray.init("local://") (without an address), which starts a local Ray cluster and connects to it via the Ray client. I don't think we should support ray.init("local://address:port") because that's already covered by ray.init("ray://address:port"). If we later want to add the functionality to start a local cluster and connect with the client, we may even want to do it with ray.init("ray://").

ckw017 · 2021-08-02T16:16:22Z

Very close to LGTM. A few final comments (mainly removing local:// from docs and the code and a NIT in the ray start output). Please make sure to file an issue (serverless milestone, P1) for the remaining pieces from these (if they are not address already):

Can we remove all the references of ray.util.connect() in the ray docs?

Can we please be very explicit that there are two ways of connecting? one from the cluster and one from outside the cluster?

can we remove the ray.client().connect() documentation?

Can we add warnings for ray.client().connect()?

1, 2 and 3 will be covered by this PR, opened #17508 for 4

python/ray/client_builder.py

doc/source/cluster/ray-client.rst

AmeerHajAli

LGTM. Left a few comments. Let me know when ready.

Co-authored-by: Ameer Haj Ali <ameerh@berkeley.edu>

ckw017 · 2021-08-02T22:25:37Z

Should be good, will drop author-action once tests are passing

doc/source/cluster/ray-client.rst

ckw017 · 2021-08-04T00:41:04Z

Looks like there are some ongoing issues with java-streaming (14% flaky) and apple tests (out of disk during build) atm : (

AmeerHajAli · 2021-08-04T01:14:18Z

Looks like there are some ongoing issues with java-streaming (14% flaky) and apple tests (out of disk during build) atm : (

Based on the test failure, is this ready to merge?

ckw017 · 2021-08-04T02:26:33Z

Should be fine to merge IMO, 6bfc8f1 was passing everything except a known flaky windows test (which is passing on the most recent run), and the only changes since that commit have been to documentation files.

AmeerHajAli · 2021-08-04T02:31:04Z

Sounds good.

ckw017 added 11 commits July 21, 2021 22:39

start

4fb2946

Merge branch 'master' of github.com:ray-project/ray into moreclientwa…

0327c10

…rnings

check formatting

416b12f

undo changes from base branch

bd13084

Client builder API docs

f7cdbdd

indent

2be24cb

8

e0b0e53

minor fixes

b509a31

absolute path to runtime env docs

51f871e

fix runtime_env link

f1808dd

Update worker.init docs

df8d09e

ckw017 commented Jul 26, 2021

View reviewed changes

ckw017 requested review from ijrsvt and AmeerHajAli July 26, 2021 06:09

AmeerHajAli reviewed Jul 26, 2021

View reviewed changes

doc/source/cluster/ray-client.rst Outdated Show resolved Hide resolved

AmeerHajAli reviewed Jul 26, 2021

View reviewed changes

ckw017 added 3 commits July 26, 2021 12:44

drop clientbuilder docs, link to 1.4.1 docs instead. Specify local://…

f4c5356

… behavior when address passed

add debug info for ray.init("local")

53e38c4

local:// attaches a driver directly

1e4ba86

AmeerHajAli assigned pcmoritz Jul 26, 2021

AmeerHajAli requested a review from pcmoritz July 26, 2021 20:48

ijrsvt reviewed Jul 27, 2021

View reviewed changes

python/ray/worker.py Outdated Show resolved Hide resolved

ijrsvt reviewed Jul 27, 2021

View reviewed changes

python/ray/worker.py Outdated Show resolved Hide resolved

ijrsvt approved these changes Jul 27, 2021

View reviewed changes

ckw017 added 2 commits July 27, 2021 15:09

update ray.init return wording

7d24d33

remote init.connect() from example

49ad340

ckw017 assigned richardliaw Jul 28, 2021

ckw017 requested a review from richardliaw July 28, 2021 17:22

AmeerHajAli reviewed Jul 29, 2021

View reviewed changes

ckw017 added 2 commits August 2, 2021 08:54

Merge branch 'newinitdocs' of github.com:ckw017/ray into newinitdocs

343ea3a

lint

50bd0cb

ckw017 removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Aug 2, 2021

ckw017 added 4 commits August 2, 2021 09:59

doc lint

e5f26aa

remove local:// codepath

31fd754

remove 'internal_config'

9b0fd84

Merge branch 'master' into newinitdocs

6bfc8f1

AmeerHajAli reviewed Aug 2, 2021

View reviewed changes

python/ray/client_builder.py Show resolved Hide resolved

AmeerHajAli reviewed Aug 2, 2021

View reviewed changes

python/ray/client_builder.py Show resolved Hide resolved

AmeerHajAli reviewed Aug 2, 2021

View reviewed changes

doc/source/cluster/ray-client.rst Outdated Show resolved Hide resolved

AmeerHajAli reviewed Aug 2, 2021

View reviewed changes

doc/source/cluster/ray-client.rst Outdated Show resolved Hide resolved

AmeerHajAli approved these changes Aug 2, 2021

View reviewed changes

AmeerHajAli added this to the Serverless Autoscaling milestone Aug 2, 2021

AmeerHajAli added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Aug 2, 2021

ckw017 and others added 2 commits August 2, 2021 15:17

Update doc/source/cluster/ray-client.rst

d7a15f7

Co-authored-by: Ameer Haj Ali <ameerh@berkeley.edu>

doc suggestion

938b103

AmeerHajAli reviewed Aug 2, 2021

View reviewed changes

doc/source/cluster/ray-client.rst Outdated Show resolved Hide resolved

AmeerHajAli and others added 5 commits August 3, 2021 01:27

Update doc/source/cluster/ray-client.rst

8f2f9f1

Merge branch 'master' into newinitdocs

1dc1554

Merge branch 'newinitdocs' of github.com:ckw017/ray into newinitdocs

bfee03c

Merge branch 'master' into newinitdocs

57b38b3

Merge branch 'master' into newinitdocs

b68c7a6

ckw017 removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Aug 4, 2021

AmeerHajAli merged commit a33cbec into ray-project:master Aug 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[client][docs] update docs for new client support in init #17333

[client][docs] update docs for new client support in init #17333

ckw017 commented Jul 26, 2021 •

edited

Loading

ckw017 Jul 26, 2021

richardliaw Jul 30, 2021

AmeerHajAli left a comment

ijrsvt left a comment

ckw017 commented Jul 28, 2021

AmeerHajAli Jul 29, 2021

AmeerHajAli Jul 29, 2021

ckw017 Jul 30, 2021 •

edited

Loading

AmeerHajAli Jul 30, 2021

ckw017 Jul 30, 2021 •

edited

Loading

AmeerHajAli Aug 1, 2021

pcmoritz Aug 2, 2021 •

edited

Loading

ckw017 commented Aug 2, 2021

AmeerHajAli left a comment

ckw017 commented Aug 2, 2021

ckw017 commented Aug 4, 2021

AmeerHajAli commented Aug 4, 2021

ckw017 commented Aug 4, 2021

AmeerHajAli commented Aug 4, 2021


		You can also connect using the ClientBuilder API, but this will eventually be deprecated.
		Local mode is used when the address is prefixed with ``local://``. If no address is provided, i.e. ``ray.init("local://")``), a local cluster is created and connected to directly by attaching a driver. If an address is provided, i.e. ``ray.init("local://localhost:6379")``, Ray will attempt to attach a driver to an existing cluster at the specified address. Local mode accepts all of the same keywords as ``ray.init`` and uses them to configure the local cluster.

[client][docs] update docs for new client support in init #17333

[client][docs] update docs for new client support in init #17333

Conversation

ckw017 commented Jul 26, 2021 • edited Loading

Why are these changes needed?

Related issue number

Checks

ckw017 Jul 26, 2021

Choose a reason for hiding this comment

richardliaw Jul 30, 2021

Choose a reason for hiding this comment

AmeerHajAli left a comment

Choose a reason for hiding this comment

ijrsvt left a comment

Choose a reason for hiding this comment

ckw017 commented Jul 28, 2021

AmeerHajAli Jul 29, 2021

Choose a reason for hiding this comment

AmeerHajAli Jul 29, 2021

Choose a reason for hiding this comment

ckw017 Jul 30, 2021 • edited Loading

Choose a reason for hiding this comment

AmeerHajAli Jul 30, 2021

Choose a reason for hiding this comment

ckw017 Jul 30, 2021 • edited Loading

Choose a reason for hiding this comment

AmeerHajAli Aug 1, 2021

Choose a reason for hiding this comment

pcmoritz Aug 2, 2021 • edited Loading

Choose a reason for hiding this comment

ckw017 commented Aug 2, 2021

AmeerHajAli left a comment

Choose a reason for hiding this comment

ckw017 commented Aug 2, 2021

ckw017 commented Aug 4, 2021

AmeerHajAli commented Aug 4, 2021

ckw017 commented Aug 4, 2021

AmeerHajAli commented Aug 4, 2021

ckw017 commented Jul 26, 2021 •

edited

Loading

ckw017 Jul 30, 2021 •

edited

Loading

ckw017 Jul 30, 2021 •

edited

Loading

pcmoritz Aug 2, 2021 •

edited

Loading