-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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] Make "import ray" work without grpcio #35737
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.
maybe it is not ready yet since we didn't merge the Pubsub PR yet, but we can probably add a unit test to verify grpc modules are not imported from some of unit tests using contest?
@@ -135,11 +134,13 @@ def set_default_actor_lifetime(self, default_actor_lifetime: str) -> None: | |||
Args: | |||
default_actor_lifetime: The default actor lifetime to set. | |||
""" | |||
import ray.core.generated.common_pb2 as common_pb2 |
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.
For the protobuf, any special reason we import inside a method?
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.
This is needed so the docs build is working (some stuff gets mocked there and if this is top level, that prevents things from working). Now that I'm thinking about this, let me actually wrap this use in Cython -- then we can get rid of this import and also wrapping it will be needed anyways going forward when we also want to get rid of the Python protobuf dependency as well :)
@@ -141,7 +140,7 @@ class PyModulesPlugin(RuntimeEnvPlugin): | |||
|
|||
name = "py_modules" | |||
|
|||
def __init__(self, resources_dir: str, gcs_aio_client: GcsAioClient): | |||
def __init__(self, resources_dir: str, gcs_aio_client): |
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.
same question as above
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 found a better way now to keep expressing the typing without needing to import the module, updated :)
Co-authored-by: SangBin Cho <[email protected]> Signed-off-by: Philipp Moritz <[email protected]>
Why are these changes needed? Import cleanups to push grpc imports out of the critical path of Ray Related issue number Next step in ray-project#35472 Signed-off-by: Philipp Moritz <[email protected]> Co-authored-by: SangBin Cho <[email protected]>
Why are these changes needed? Import cleanups to push grpc imports out of the critical path of Ray Related issue number Next step in ray-project#35472 Signed-off-by: Philipp Moritz <[email protected]> Co-authored-by: SangBin Cho <[email protected]> Signed-off-by: e428265 <[email protected]>
Why are these changes needed?
Import cleanups to push
grpc
imports out of the critical path of RayRelated issue number
Next step in #35472
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.