[Serve] [3/n] Add ClientStreaming and BidiStreaming RPCs to proto definitions#60769
[Serve] [3/n] Add ClientStreaming and BidiStreaming RPCs to proto definitions#60769abrarsheikh merged 11 commits intomasterfrom
Conversation
This PR adds the foundational types needed for gRPC client streaming and bidirectional streaming support: - gRPCStreamingType enum: Represents the four gRPC streaming types (UNARY_UNARY, UNARY_STREAM, STREAM_UNARY, STREAM_STREAM) - gRPCStreamingRequest dataclass: Carries metadata about streaming sessions from proxy to replicas - gRPCInputStream class: Public API for deployments to iterate over incoming request messages in client/bidirectional streaming RPCs This is PR 1 of N for adding gRPC bidirectional streaming support. Signed-off-by: abrar <abrar@anyscale.com>
This PR refactors the gRPC server's service handler factory to use the new gRPCStreamingType enum instead of a boolean stream parameter. Changes: - Update gRPCGenericServer.add_generic_rpc_handlers() to register all four streaming handler types (unary_unary, unary_stream, stream_unary, stream_stream) using gRPCStreamingType enum - Update unit tests to verify all four handler types are registered This is PR 2 of N for adding gRPC bidirectional streaming support. Signed-off-by: abrar <abrar@anyscale.com>
This PR adds the proto definitions for client streaming and bidirectional streaming RPCs: - Add ClientStreaming (stream-unary) RPC to UserDefinedService - Add BidiStreaming (stream-stream) RPC to UserDefinedService - Regenerate Python protobuf files for documentation examples This is PR 3 of N for adding gRPC bidirectional streaming support. Signed-off-by: abrar <abrar@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for client-side and bidirectional gRPC streaming to Ray Serve, which is a valuable feature enhancement. The changes are well-structured, including updates to protobuf definitions, the introduction of a gRPCStreamingType enum for clarity, and a new gRPCInputStream public API for handling streaming requests. The internal logic, tests, and documentation have been updated accordingly. The implementation appears solid, and the new APIs are well-designed. I have one minor suggestion to improve an example in a docstring to avoid potential user confusion.
| async def ClientStreaming(self, request_stream: gRPCInputStream): | ||
| total = 0 | ||
| async for request in request_stream: | ||
| total += request.value |
There was a problem hiding this comment.
The example code in the docstring uses request.value, but the UserDefinedMessage protobuf message typically used in examples doesn't have a value field. It has name, origin, and num. To make the example clearer and more consistent with other examples, please use an actual field from the message, like request.num.
| total += request.value | |
| total += request.num |
abrarsheikh
force-pushed
the
abrar-grpc-bidir-pr2
branch
from
30623c9 to
235c1c7
Compare
abrarsheikh
force-pushed
the
abrar-grpc-bidir-pr3
branch
from
b35dcbf to
038cafc
Compare
Signed-off-by: abrar <abrar@anyscale.com>
Signed-off-by: abrar <abrar@anyscale.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| rpc Multiplexing(UserDefinedMessage2) returns (UserDefinedResponse2); | ||
| rpc Streaming(UserDefinedMessage) returns (stream UserDefinedResponse); | ||
| rpc ClientStreaming(stream UserDefinedMessage) returns (UserDefinedResponse); | ||
| rpc BidiStreaming(stream UserDefinedMessage) returns (stream UserDefinedResponse); |
There was a problem hiding this comment.
Proto file modified, requires fault-tolerance review
Low Severity
.proto files.
Please review the RPC fault-tolerance & idempotency standards guide here:
https://github.com/ray-project/ray/tree/master/doc/source/ray-core/internals/rpc-fault-tolerance.rst
This violates the "RPC Fault Tolerance Standards Guide" rule requiring this message whenever .proto files are changed.
Additional Locations (1)
| rpc Multiplexing(UserDefinedMessage2) returns (UserDefinedResponse2); | ||
| rpc Streaming(UserDefinedMessage) returns (stream UserDefinedResponse); | ||
| rpc ClientStreaming(stream UserDefinedMessage) returns (UserDefinedResponse); | ||
| rpc BidiStreaming(stream UserDefinedMessage) returns (stream UserDefinedResponse); |
There was a problem hiding this comment.
PR lacks description explaining changes
Low Severity
To help reviewers, please ensure your PR includes:
- Title: A concise summary of the change
- Description:
- What problem does this solve?
- How does this PR solve it?
- Any relevant context for reviewers such as:
- Why is the problem important to solve?
- Why was this approach chosen over others?
See this list of PRs as examples for PRs that have gone above and beyond:
- [Core] Introduce local port service discovery #59613
- [Core] Improve Large-Scale Resource View Synchronization Through Sync Message Batching #57641
- Remove node observability information from hot path of core components #56474
- [core][rdt] Support out-of-order actors by extracting metadata when creating #59610
- [core] fix open leak for plasma store memory (shm/fallback) by workers #52622
This violates the "Clear PR Descriptions and Titles" rule because the description is "No description provided."
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Ondrej Prenek <ondra.prenek@gmail.com>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Muhammad Saif <2024BBIT200@student.Uet.edu.pk>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Adel Nour <ans9868@nyu.edu>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
…initions (ray-project#60769) Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
2 participants
No description provided.