Reversing Dependency: Implementing Untyped-RPC on Top of Typed-RPC (draft) #51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- `Grpc_protobuf` is no longer eio specific
- Make use of ocaml-protoc's server implementation bundle as intended; - Adapt routeguide example to ocaml-protoc to show the differences. This results in a more complicated [Grpc.Rpc] interface that tries to better capture the common parts between ocaml-protoc and ocaml-protoc-plugin that can be used by ocaml-grpc.
This reverts commit 042efeb.
- This adds type safety to ensure the RPCs are called with the expected protocol (e.g. you cannot call a unary rpc with a server_streaming entry point, etc.). On the ocaml-protoc-plugin side, currently there are no markers for the rpc modes - this interface will permit adding them in the future without user facing changes. On the ocaml-protoc plugin, the value mode flows from the proto file definition and is checked in the user code as expected. Implementation note: There's perhaps a way to shorten the mapping of value-modes but I couldn't find one given that `Grpc` cannot depend on `Ocaml_protoc`, and thus the `Value_mode` types are not equal.
In the existing examples, the service name is separated from the package name by a dot, which I inadvertently omitted in the previous implementation. Note that as long as the service name used by a client and a server is the same, the right handler is executed, so there's some leeway in the actual choice of the convention to use. The hope is that the dot separated one is standard.
|
Currently this PR is built on top of #48 thus duplicating the commit chain.
|
|
@quernd and @tmcgilchrist I prefer this implementation over the one in #48. If you like it too, I will use this as the basis for the typed-rpc-eio implementation that I enable for code review. Please let me know what you think. Thank you! |
mbarbin
changed the title
|
I ended up re-using this code organization in #55. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this Pull Request, I've reversed the dependency between the typed-RPC and
untyped-RPC implementations. Now, the untyped-RPC is built on top of the
typed-RPC, a shift from the previous structure.
This change is motivated by several factors:
It paves the way for a straightforward deprecation of the untyped-RPC in the
future, as we can simply remove the code when ready.
It reduces indirection and allocation in the typed-RPC implementation path.
With this new implementation, there's no need to map over sequences, which
should enhance the performance of the typed implementation and clarify the code.
While this modification isn't strictly necessary for the PR introducing the
typed-RPC interface (#48), I believe it's a valuable addition. This approach aligns
with my original vision during the exploration phase, and I recommend
implementing it without significant delay. Specifically, I suggest that the
typed interface implementation for other concurrency libraries should follow a
similar path, bypassing the initial exploratory phases of building the typed-RPC
on top of the untyped one.