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

Add services dynamically with tonic #886

Closed
Miaxos opened this issue Jan 11, 2022 · 6 comments
Closed

Add services dynamically with tonic #886

Miaxos opened this issue Jan 11, 2022 · 6 comments

Comments

@Miaxos
Copy link

Miaxos commented Jan 11, 2022 •
edited
Loading

Feature Request

Hello 👋!

Thread from discord which motivated this issue creation: https://discord.com/channels/500028886025895936/628706823104626710/930507181516795944

Motivation

I want to dynamically add services where those services would be initialized later.

For instance, let's suppose we want a Builder pattern to create GRPC services based on a Shared Context where you could register your Service creation function.

struct GRPCDriver<C, Se>
where
    C: BusinessContext + Send + Sync + 'static,
    Se: Service<hyper::Request<Body>, Response = Response<BoxBody>>
        + NamedService
        + Clone
        + Send
        + 'static,
    Se::Future: Send + 'static,
    Se::Error: Into<TonicError> + Send,
{
    // Here we have our "service factory" which create services based on a Shared Context
    grpc_services: Vec<Box<dyn Fn(Arc<C>) -> Se + Send>>,
    ..
}

impl<C, Se> GRPCDriver<C, Se>
where
    C: BusinessContext + Send + Sync + 'static,
    Se: Service<Request<Body>, Response = Response<BoxBody>>
        + NamedService
        + Clone
        + Send
        + 'static,
    Se::Future: Send + 'static,
    Se::Error: Into<TonicError> + Send,
{

   // Here the function which will run the server and services initialization.
  async fn run_driver_without_context(self, context: Arc<C>) -> anyhow::Result<()> {

    let grpc_services: Vec<Box<dyn Fn(Arc<C>) -> Se + Send>> = self.grpc_services;
    let mut server = TonicServer::builder();

    for service in grpc_services {
        server = server.add_service(service(Arc::clone(&context)));
        // Here types are fucked up because the compiler can't infer the right type for server.
    }

    server.await

  }
}

It won't work due to the typing of the successive add_service call.

Proposal

Not really :(
@LucioFranco
Copy link
Member

Unless axum supports this (cc @davidpdrsn ) its likely we won't add this support since its likely to either add perf issues or complexity.

@davidpdrsn
Copy link
Member

Axum makes it a bit easier (once the Router type doesn't change as you add routes) but a vec of closures that returns generic services is going to be hard no matter what I think.

I agree this probably isn't something we want to support since it's likely to add more complexity.

@Miaxos
Copy link
Author

Miaxos commented Jan 13, 2022

Yeah I understand, it does add a lot of complexity. I'm working on a solution that would implies a public expose of Or and Routes (Miaxos/tonic@master...hyperium:bb392be46ed6708d1282dd639c53a40d53c6c9fc) struct which is kinda working right now, I've still things to check, test and fix. If I succeed in doing what I want I'll keep you informed.

I'm closing this as it won't be implemented.
Thank you

@Miaxos Miaxos closed this as completed Jan 13, 2022
@davidpdrsn
Copy link
Member

Thanks for understanding :)

@Miaxos Miaxos mentioned this issue Jan 18, 2022
Closed
@alamb
Copy link

alamb commented Mar 7, 2022

FWIW I think the change from @davidpdrsn in #830 may help this ticket significantly (because the Router no longer has so many generic types)

@alamb
Copy link

alamb commented Mar 10, 2022

Specifically, in the above example

    for service in grpc_services {
        // this type is `Router<Routes>`
        server = server.add_service(service(Arc::clone(&context)));
    }

@MBoui MBoui mentioned this issue Jun 1, 2023
Open
@tsroka tsroka mentioned this issue Jun 6, 2023
Merged
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
4 participants
@alamb @davidpdrsn @Miaxos @LucioFranco