feat(grpc): add ServerBuilder, registration, interceptors, and routing - #2789
feat(grpc): add ServerBuilder, registration, interceptors, and routing#2789sauravzg wants to merge 2 commits into
Conversation
f7320bc to
98ab999
Compare
98ab999 to
a682691
Compare
74c4469 to
cbedd8d
Compare
Introduce the server-side handle/router API for building a gRPC `Server` from a fluent builder. - ServerBuilder: fluent construction via `Server::builder()`, with `add_service`, `interceptor`and `build` / `build_with_runtime`. - Service + ServiceExt: `Service` trait for method registration, plus `with_interceptor` to wrap all of a service's methods (InterceptedService). - Interceptors: `Intercept` trait, no-op `Identity`, and `InterceptExt::chain` for composing interceptors into an `InterceptorChain` (first added runs outermost). - Descriptors: `ServiceDescriptor`, `MethodDescriptor`, and `MethodType`. - Routing: `RouterBuilder` maps method paths to `DynHandle`s. - Options: `ServerOptions` currently empty, but a kitchen sink for options.
cbedd8d to
f3ce938
Compare
|
|
||
| /// The type (cardinality) of a gRPC method. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum MethodType { |
There was a problem hiding this comment.
I thought we would hide this from grpc itself and let it treat everything as bidi streaming?
There was a problem hiding this comment.
Sorry, I missed addressing this yesterday. I agree in principle but I think we have precedent at least in java and very likely c++(but not c-core), where the descriptors are a part of interceptor API and support a notion of method type.
Slighly confused about go , but you probably know better about it.
Given, that this is very useful information to have about an RPC, I am leaning towards the Java-ish approach which would essentially be adding another enum named "unknown" here and be done with it.
| rx: impl RecvStream + 'static, | ||
| next: &impl Handle, | ||
| ) -> Trailers { | ||
| next.handle(headers, options, tx, rx).await |
There was a problem hiding this comment.
With optimizations on, does this cause literally no overhead?
There was a problem hiding this comment.
I can't confirm this , but I don't see a reason why. It's a zero sized type with no allocations and I'd assume the compiler can inline this to make it no-op.
Should we add a microbenchmark for this?
| /// Creates a new server with the given handler, runtime, and options. | ||
| pub(crate) fn new( |
There was a problem hiding this comment.
Shouldn't we delete new and require the use of the builder?
There was a problem hiding this comment.
new is pub(crate) , the only public way to construct is via the builder which on build calls this new.
Are we looking for something different?
| use crate::server::service::Service; | ||
|
|
||
| /// A builder for constructing an immutable [`Router`]. | ||
| pub(crate) struct RouterBuilder<I = Identity> { |
There was a problem hiding this comment.
Hmm, why not use a default type for ServerBuilder, too? I assume this is here so that you can impl RouterBuilder instead of needing to impl RouterBuilder<Identity>?
There was a problem hiding this comment.
ServerBuilder is already default typed. The chain of default types was keeping everything similar .
Anyways, followed the call chain and realized that it's not needed, since the way it's modelled right now is to chain interceptors and the only public entrypoint is Server::builder(), having default type on ServerBuilder combined with impl RouterBuilder<Identity> should be good enough.
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ```ignore |
There was a problem hiding this comment.
I would prefer if we could avoid ignore for our examples. Especially as we're heavily iterating on things, there's a high likelihood that they will become broken this way. Please use other tags instead like no_run or use leading # to hide things from the rendered documentation.
There was a problem hiding this comment.
I've decided to remove most of the doc tests. Didn't see a huge value in the ones with no_run .
Fixing them right now would make a bunch of symbols public which I would want to avoid until it's needed.
2 participants
Introduce the server-side handle/router API for building a gRPC
Serverfrom a fluent builder.Server::builder(), withadd_service,interceptorandbuild.Servicetrait for method registration, pluswith_interceptorto wrap all of a service's methods (InterceptedService).Intercepttrait, no-opIdentity, andInterceptExt::chainfor composing interceptors into anInterceptorChain(first added runs outermost).ServiceDescriptor,MethodDescriptor, andMethodType.RouterBuildermaps method paths toDynHandles.ServerOptionscurrently empty, but a kitchen sink for options.