feat: port slimrpc code to rust lang #1036
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
sancyx
force-pushed
the
slimrpc-rust
branch
3 times, most recently
from
6b8f05b to
15c60c1
Compare
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
data-plane/slimrpc/src/server.rs
Outdated
| responses.push(response?); | ||
| } | ||
|
|
||
| Ok(responses) |
There was a problem hiding this comment.
This isn't quite right, what will happen here is that the stream will get drained into responses, and then returned to handle_session, which will then drain it into the SLIM channel.
Instead call_handler should return the stream itself to handle_session, and as items are drained from the stream they should be sent on the SLIM channel.
In python we handled this by having call_handler return a generator of responses regardless of stream or not stream.
Immediate thought is that call_handler should create a channel, and a background task to do the handling, then return the channel to session_handler or perhaps we can use a futures:Stream.
Honestly I would revisit this whole logic as we're in a different language for example:
session_handler can create two different channels one for inputs and one for responses, these are passed to call_handler:
For UnaryUnary:
- session_handler reads one message and push it down the input channel
- call_hander reads one message from the input channel
- calls the handler
- sends the response down the output channel then closes it
For UnaryStream:
- session_handler reads one message and push it down the input channel
- call_hander reads one message from the input channel
- calls the handler
- drains the response stream down the output channel then closes it
For StreamUnary:
- session_handler creates a routine/task which continuously reads messages and pushes them down the input channel
- call_handler passes the stream to the handler
- waits for one response from the call_handler function
- sends the response down the output channel then closes it
For StreamStream:
- session_handler creates a routine/task which continuously reads messages and pushes them down the input channel
- call_handler passes the stream to the handler
- drains the response stream down the output channel then closes it
session_handler's main "thread" will drain the output channel, forwarding each message, and then when the channel closes / ends it will send the channel end response to the client.
| msg_ctx: MessageContext, | ||
| session_ctx: SessionContext, | ||
| ) -> Result<Res>; | ||
| } |
There was a problem hiding this comment.
In order for this to work with uniffi, we need to make sure that we enable the trait to be implemented by the foreign/binding language explictly: https://mozilla.github.io/uniffi-rs/latest/foreign_traits.html
| msg_ctx: MessageContext, | ||
| session_ctx: SessionContext, | ||
| ) -> Result<ResponseStream<Res>>; | ||
| } |
There was a problem hiding this comment.
I'm not sure that the function declaration here is correct, in python we had call implement a "generator" essentially the function gets wrapped by the runtime into an iterator class which isn't a concept that exists in rust. This current declaration would require the call function to create and return a stream and trigger downstream a background task to populate the stream.
It would be better for Stream handlers to receive an output stream channel rx from the session handler, so that they can simply call output_stream.try_send(...) or add we function on the session_ctx to send responses which then pushes responses onto the channel internally (this is how gRPC implements it BTW).
There was a problem hiding this comment.
For reference this is the gRPC interface for the session context:
https://github.com/grpc/grpc/blob/master/src/python/grpcio/grpc/aio/_base_server.py#L154
read is used to read messages for "Stream****" channels and write is used to response to "***Stream" channels.
Signed-off-by: Magyari Sandor Szilard <[email protected]>
Signed-off-by: Magyari Sandor Szilard <[email protected]>
3 participants
Description
Slimrpc code ported from python to Rust.
Related issue: #1005
Type of Change
Checklist