Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Using non-NamedService in Server builder #1305

Closed
danielsig727 opened this issue Mar 8, 2023 · 3 comments
Closed

Using non-NamedService in Server builder #1305

danielsig727 opened this issue Mar 8, 2023 · 3 comments

Comments

@danielsig727
Copy link

Hello,

TLDR: I would like to have a way to add a service not implementing NamedService. So the service name could be given at runtime, not hard-coded as a const.

Context

I'm trying to implement a server to receive requests from runtime-decided or arbitrary service names sharing the same req&resp proto definition. This sounds not too hard from an HTTP point of view -- I hope the same tower::Service, without implementing NamedService, can be used for multiple service names.

I'm currently using the builder like most examples:

    tonic::transport::Server::builder()
        .add_service(my_service)
        .serve(addr)
        .await?;

with the my_service being a modified copy from the output of codegen. But I was blocked by the fact that tonic::server::NamedService requires a hard-coded server name,

/// A trait to provide a static reference to the service's
/// name. This is used for routing service's within the router.
pub trait NamedService {
    /// The `Service-Name` as described [here].
    ///
    /// [here]: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#requests
    const NAME: &'static str;
}

And in router, add_service is the only method to configure the router depending on NamedService

    pub(crate) fn add_service<S>(mut self, svc: S) -> Self
    where
        S: Service<Request<Body>, Response = Response<BoxBody>, Error = Infallible>
            + NamedService
            + Clone
            + Send
            + 'static,
        S::Future: Send + 'static,
        S::Error: Into<crate::Error> + Send,
    {
        let svc = svc.map_response(|res| res.map(axum::body::boxed));
        self.router = self
            .router
            .route_service(&format!("/{}/*rest", S::NAME), svc);
        self
    }

Possible Solutions

I've thought of a few possible solns to this

  1. expose the axum router publicly => maybe not so good, due to it leaks the implementation
  2. add another method similar to add_service but not relying on NamedService. so it could be something like
tonic::transport::Server::builder().add_service_with_name(svc_name, my_service)

With svc_name being decided at runtime.
3. Maybe more specific to my use case, it would also work if there's a way to replace the unimplemented fallback with my own service (so it handles all endpoints by default). Probably with something like

tonic::transport::Server::builder().with_fallback(my_service)

I can help create the patch either way. But before i do so I would really appreciate your feedback first.
Or are there other ways to implement this with the current API I missed? Thank you for your time!
@Palmik
Copy link

Palmik commented Mar 12, 2023

Checkout https://github.com/hyperium/tonic/blob/master/examples/src/grpc-web/server.rs#L42 and #1177 -- the docs seem out of date

@danielsig727
Copy link
Author

thanks @Palmik I've checked but still not sure how layer() could help? since it still seems to depend on add_service(svc) later on as the example, which still requires to be a NamedService. And I'm not suing grpc-web

@LucioFranco
Copy link
Member

So the namedservice stuff is mainly used by the tonic router, if you want to route stuff dynamically you will need to build your own transport that does its own routing. Currently, supporting dynamic services out of the box in tonic is a non-goal as eventually the transport module will be removed infavor of a better version, so making large changes like this won't be accepted right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Palmik @danielsig727 @LucioFranco