-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
feat(transport): Add service multiplexing/routing #99
Conversation
This change introduces a new "router" built on top of `transport::Server` that allows one to run multiple gRPC services on the same socket. ```rust Server::builder() .add_service(greeter) .add_service(echo) .serve(addr) .await?; ``` There is also a new `multiplex` example showcasing server side service multiplexing and client side service multiplexing. BREAKING CHANGES: `Server::serve` is now crate private and all services must be added via `Server::add_service`. Codegen also returns just a `Service` now instead of a `MakeService` pair. Closes #29 Signed-off-by: Lucio Franco [email protected]
@@ -13,7 +13,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> { | |||
let client_key = tokio::fs::read("tonic-examples/data/tls/client1.key").await?; | |||
let client_identity = Identity::from_pem(client_cert, client_key); | |||
|
|||
let tls = ClientTlsConfig::with_openssl() | |||
let tls = ClientTlsConfig::with_rustls() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this supposed to change in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I think it was missed in another PR so I thought i might as well change this now. I want all examples with rustls :)
We really only need openssl for interop since the certs that grpc-go uses are too small for the min that rustls supports.
/// | ||
/// This will clone the `Server` builder and create a router that will | ||
/// route around different services. | ||
pub fn add_service<S>(&mut self, svc: S) -> Router<S, Unimplemented> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need to take &mut self
if it immediately clones self
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because you can create multiple routers from one Server
config was the idea. It takes a &mut because it follows like the others.
Ok(()).into() | ||
} | ||
|
||
fn call(&mut self, _req: Request<Body>) -> Self::Future { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think this should record a tracing event as well? that way, users get some visibility into when requests hit the unimplemented route (in logs, metrics, etc...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ummm probably a good idea, but I'd like to add tracing in another PR.
impl<A, B, Request> Routes<A, B, Request> { | ||
pub(crate) fn push<C>( | ||
self, | ||
predicate: impl Fn(&Request) -> bool + Send + Sync + 'static, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth considering a Predicate
trait so that users can add more complex filtering? Probably not in this branch though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, once we get a stable release of tokio, im gonna update a bunch of the tower services and will impl this there.
* feat(transport): Add service multiplexing/routing This change introduces a new "router" built on top of `transport::Server` that allows one to run multiple gRPC services on the same socket. ```rust Server::builder() .add_service(greeter) .add_service(echo) .serve(addr) .await?; ``` There is also a new `multiplex` example showcasing server side service multiplexing and client side service multiplexing. BREAKING CHANGES: `Server::serve` is now crate private and all services must be added via `Server::add_service`. Codegen also returns just a `Service` now instead of a `MakeService` pair. Closes hyperium#29 Signed-off-by: Lucio Franco [email protected]
This change introduces a new "router" built on top of
transport::Server
that allows one to run multiplegRPC services on the same socket.
Example
There is also a new
multiplex
example showcasingserver side service multiplexing and client side
service multiplexing.
BREAKING CHANGES:
Server::serve
is now crate privateand all services must be added via
Server::add_service
.Codegen also returns just a
Service
now instead of aMakeService
pair.Closes #29
cc @rlabrecque @seanmonstar @carllerche
Signed-off-by: Lucio Franco [email protected]