[no_early_kickoff] Make sure all public APIs will auto-init ray #34730

raulchen · 2023-04-24T22:18:58Z

Why are these changes needed?

Fix the issue that get_runtime_context won't auto-init ray.
Make sure all public APIs will auto-init ray, by decorating all functions in __all__ of ray/__init__.py.
Decouple auto-init with client mode.

Related issue number

Fixes #34631

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

Signed-off-by: Hao Chen <chenh1024@gmail.com>

pcmoritz · 2023-04-24T22:25:14Z

Nice!

ericl

Cool. Consider adding a unit tests / automated checks for the symbols exposed in init as you mentioned earlier.

Btw, our convention is to "assign" reviewers instead of "request reviewers". This is because assignees show up persistently in github.com/pulls, but review requests sometimes appear and disappear so it's not a great way to track what PRs you're supposed to review.

Signed-off-by: Hao Chen <chenh1024@gmail.com>

raulchen · 2023-04-25T02:42:21Z

@pcmoritz @ericl I just reimplemented the code and make sure all APIs are wrapped. Do you want to take a look again?

raulchen · 2023-04-25T02:39:12Z

python/ray/_private/storage.py

@@ -25,7 +26,8 @@
 _filesystem = None


-@client_mode_hook(auto_init=True)
+@wrap_auto_init


This api (along with get_client and list_named_actors) isn't part of __init__.py:__all__.
Not sure if they should be there. I didn't move them to __all__, but just added a standalone wrapper there.

Sounds good. They are not public APIs yet, but the plan is to eventually get them there.

pcmoritz · 2023-04-25T03:39:46Z

If you want to auto-wrap the APIs, would it be better to have a separate list? Something like

AUTO_INIT_RAY_APIS = [
...
]

wrap_auto_init_for_all_apis(AUTO_INIT_RAY_APIS)

__all__ += AUTO_INIT_RAY_APIS

We probably don't want things like ObjectID to auto-init (which I think your change is doing?), what do you think?

raulchen · 2023-04-25T18:22:53Z

@pcmoritz that makes sense. Currently I avoid ObjectID by only only wrapping function objects.

Signed-off-by: Hao Chen <chenh1024@gmail.com>

pcmoritz

The new scheme makes sense I think.

I do enourage you to put it into ray/__init__.py like

AUTO_INIT_APIS = [
    "cancel",
    "get",
    "get_actor",
    "get_gpu_ids",
    "get_runtime_context",
    "kill",
    "put",
    "wait",
]

wrap_auto_init_for_apis(AUTO_INIT_APIS)

since that makes it a little more discoverable for adding new APIs :)

Signed-off-by: Hao Chen <chenh1024@gmail.com>

ericl · 2023-04-26T22:00:46Z

python/ray/__init__.py

+    "kill",
+    "put",
+    "wait",
+]


Hmm, I think it's actually a bug that cluster_resources / available_resources isn't enabled in auto init. To make sure folks don't forgot one or the other, how about we make this explicit, and have:

AUTO_INIT_APIS = [...]
NO_AUTO_INIT_APIS = [...]

And raise an error if there is some API that is not in one of these lists?

How can we collect the full list of all APIs? I checked @PublicAPI. There are too many. And most of them are not Ray core APIs.

I think it's actually a bug that cluster_resources / available_resources isn't enabled in auto init.

Which bug do you refer to? The bug reported in #34631 is because get_runtime_context doesn't auto init. "cluster_resources / available_resources" still don't auto init. Not sure whether they should.

To clarify, I mean the previous list of APIs in "all", the list of 20 or so things including ObjectID.

sounds good. Refactored the "init.py" file. do you want to take a another look?

Signed-off-by: Hao Chen <chenh1024@gmail.com>

ericl · 2023-05-01T17:34:11Z

TypeError: client_mode_hook() missing 1 required keyword-only argument: 'auto_init'

There are a number of tests failing with this currently.

Signed-off-by: Hao Chen <chenh1024@gmail.com>

raulchen · 2023-05-03T00:54:47Z

It turns out that we cannot simply auto init get_runtime_context. Because it may be called on the dashboard. I wrapped the data read api with auto init instead to fix the issue.

- Fix the issue that `get_runtime_context` won't auto-init ray. - Make sure all public APIs will auto-init ray, by decorating all functions in `__all__` of `ray/__init__.py`. - Decouple auto-init with client mode.

…mote" (#36334) #34730 changed `__all__` in `__init__.py` to a computed list. mypy doesn't support this. Reverting it back to a literal list to fix this issue. Repro: `mypy --strict --follow-imports=skip --ignore-missing-imports 1.py` ```python # 1.py import ray @ray.remote def foo() -> None: pass ``` Note, the reason why CI didn't catch this issue is because we don't enable strict mode, see [here](https://github.com/ray-project/ray/blob/407062499038344b0f3edb54b2c7985c3320d1ec/ci/lint/format.sh#L135). Currently we have too many issues if enabling strict mode. Signed-off-by: Hao Chen <chenh1024@gmail.com>

…mote" (ray-project#36334) ray-project#34730 changed `__all__` in `__init__.py` to a computed list. mypy doesn't support this. Reverting it back to a literal list to fix this issue. Repro: `mypy --strict --follow-imports=skip --ignore-missing-imports 1.py` ```python # 1.py import ray @ray.remote def foo() -> None: pass ``` Note, the reason why CI didn't catch this issue is because we don't enable strict mode, see [here](https://github.com/ray-project/ray/blob/407062499038344b0f3edb54b2c7985c3320d1ec/ci/lint/format.sh#L135). Currently we have too many issues if enabling strict mode. Signed-off-by: Hao Chen <chenh1024@gmail.com>

…mote" (#36334) (#36356) #34730 changed `__all__` in `__init__.py` to a computed list. mypy doesn't support this. Reverting it back to a literal list to fix this issue. Repro: `mypy --strict --follow-imports=skip --ignore-missing-imports 1.py` ```python # 1.py import ray @ray.remote def foo() -> None: pass ``` Note, the reason why CI didn't catch this issue is because we don't enable strict mode, see [here](https://github.com/ray-project/ray/blob/407062499038344b0f3edb54b2c7985c3320d1ec/ci/lint/format.sh#L135). Currently we have too many issues if enabling strict mode. Signed-off-by: Hao Chen <chenh1024@gmail.com>

…mote" (ray-project#36334) ray-project#34730 changed `__all__` in `__init__.py` to a computed list. mypy doesn't support this. Reverting it back to a literal list to fix this issue. Repro: `mypy --strict --follow-imports=skip --ignore-missing-imports 1.py` ```python # 1.py import ray @ray.remote def foo() -> None: pass ``` Note, the reason why CI didn't catch this issue is because we don't enable strict mode, see [here](https://github.com/ray-project/ray/blob/407062499038344b0f3edb54b2c7985c3320d1ec/ci/lint/format.sh#L135). Currently we have too many issues if enabling strict mode. Signed-off-by: Hao Chen <chenh1024@gmail.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

raulchen requested a review from ericl April 24, 2023 22:18

auto-init get_runtime_context

9a10dff

Signed-off-by: Hao Chen <chenh1024@gmail.com>

raulchen force-pushed the auto-init branch from b7ead08 to 9a10dff Compare April 24, 2023 22:20

ericl approved these changes Apr 24, 2023

View reviewed changes

ericl self-assigned this Apr 24, 2023

raulchen requested review from fishbone, stephanie-wang and suquark as code owners April 25, 2023 02:36

auto wrap all apis

72e9d4f

Signed-off-by: Hao Chen <chenh1024@gmail.com>

raulchen force-pushed the auto-init branch from 9d3384c to 72e9d4f Compare April 25, 2023 02:36

raulchen changed the title ~~Make get_runtime_context auto-init ray~~ Make sure all public APIs will auto-init ray Apr 25, 2023

raulchen commented Apr 25, 2023

View reviewed changes

ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Apr 25, 2023

white list

bbbc865

Signed-off-by: Hao Chen <chenh1024@gmail.com>

pcmoritz approved these changes Apr 25, 2023

View reviewed changes

raulchen added 5 commits April 25, 2023 14:46

refine

3481640

Signed-off-by: Hao Chen <chenh1024@gmail.com>

fix

fa7c61f

Signed-off-by: Hao Chen <chenh1024@gmail.com>

fix

feee5d8

Signed-off-by: Hao Chen <chenh1024@gmail.com>

fix

1578e66

Signed-off-by: Hao Chen <chenh1024@gmail.com>

Merge branch 'master' into auto-init

991e2a9

ericl reviewed Apr 26, 2023

View reviewed changes

raulchen added 4 commits April 28, 2023 09:48

Merge branch 'master' into auto-init

6ae8089

refine __init__.py

89a6ec3

Signed-off-by: Hao Chen <chenh1024@gmail.com>

lint

fbb6267

Signed-off-by: Hao Chen <chenh1024@gmail.com>

Merge branch 'master' into auto-init

1cd013b

ericl approved these changes May 1, 2023

View reviewed changes

raulchen force-pushed the auto-init branch from df77964 to 1cd013b Compare May 1, 2023 18:05

lint

913a31f

Signed-off-by: Hao Chen <chenh1024@gmail.com>

raulchen changed the title ~~Make sure all public APIs will auto-init ray~~ [no_early_kickoff] Make sure all public APIs will auto-init ray May 1, 2023

raulchen added 9 commits May 1, 2023 12:01

Merge branch 'master' into auto-init

9ace0bb

fix

ca901e9

Signed-off-by: Hao Chen <chenh1024@gmail.com>

fix

e2c82c8

Signed-off-by: Hao Chen <chenh1024@gmail.com>

fix

db73a73

Signed-off-by: Hao Chen <chenh1024@gmail.com>

refine

defe2f6

Signed-off-by: Hao Chen <chenh1024@gmail.com>

Merge branch 'master' into auto-init

c984239

fix

259b04e

Signed-off-by: Hao Chen <chenh1024@gmail.com>

revert disable_auto_init

2abddd4

Signed-off-by: Hao Chen <chenh1024@gmail.com>

auto init read_api

e91bb20

Signed-off-by: Hao Chen <chenh1024@gmail.com>

raulchen requested review from scv119, c21, amogkam, scottjlee and bveeramani as code owners May 2, 2023 20:15

Merge branch 'master' into auto-init

81289ba

raulchen merged commit 3a6d2a0 into ray-project:master May 3, 2023

raulchen deleted the auto-init branch May 3, 2023 17:03

messense mentioned this pull request Jun 9, 2023

Ray 2.5.0 breaks mypy #36252

Closed

raulchen mentioned this pull request Jun 12, 2023

Fix mypy error: Module "ray" does not explicitly export attribute "remote" #36334

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[no_early_kickoff] Make sure all public APIs will auto-init ray #34730

[no_early_kickoff] Make sure all public APIs will auto-init ray #34730

raulchen commented Apr 24, 2023 •

edited

Loading

pcmoritz commented Apr 24, 2023

ericl left a comment

raulchen commented Apr 25, 2023

raulchen Apr 25, 2023 •

edited

Loading

ericl Apr 25, 2023

pcmoritz commented Apr 25, 2023 •

edited

Loading

raulchen commented Apr 25, 2023

pcmoritz left a comment

ericl Apr 26, 2023

raulchen Apr 28, 2023

ericl Apr 28, 2023

raulchen May 1, 2023

ericl commented May 1, 2023

raulchen commented May 3, 2023

[no_early_kickoff] Make sure all public APIs will auto-init ray #34730

[no_early_kickoff] Make sure all public APIs will auto-init ray #34730

Conversation

raulchen commented Apr 24, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

pcmoritz commented Apr 24, 2023

ericl left a comment

Choose a reason for hiding this comment

raulchen commented Apr 25, 2023

raulchen Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

ericl Apr 25, 2023

Choose a reason for hiding this comment

pcmoritz commented Apr 25, 2023 • edited Loading

raulchen commented Apr 25, 2023

pcmoritz left a comment

Choose a reason for hiding this comment

ericl Apr 26, 2023

Choose a reason for hiding this comment

raulchen Apr 28, 2023

Choose a reason for hiding this comment

ericl Apr 28, 2023

Choose a reason for hiding this comment

raulchen May 1, 2023

Choose a reason for hiding this comment

ericl commented May 1, 2023

raulchen commented May 3, 2023

raulchen commented Apr 24, 2023 •

edited

Loading

raulchen Apr 25, 2023 •

edited

Loading

pcmoritz commented Apr 25, 2023 •

edited

Loading