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.

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 Router::into_service #419

Merged
merged 4 commits into from
Aug 20, 2020
Merged

feat(transport): Add Router::into_service #419

merged 4 commits into from
Aug 20, 2020

Conversation

tjtelan
Copy link
Contributor

@tjtelan tjtelan commented Jul 30, 2020

Motivation

Coincidentally I was going to open an issue for this after spending some time on my own trying to accomplish this, but #410 was on the first page.

I wanted to help merge #272 since it appeared to be slightly stalled. I have a need for the functionality of running an HTTP server and a gRPC server with multiple services on the same port.

Solution

The major work appeared to be complete and was appeared to be an acceptable implementation. The finishing code was actually provided in #272 (comment) so I just barely provided work. It was a successful group effort overall.

I rebased the PR branch w/ respect to the original author @cthulhua (with a minor change the import of futures_util::Future::Either) and moved the service function into the Router impl as per the original feedback.

Testing

Additionally, I added a new example hyper_warp_multiplex, a mix of the hyper_warp and multiplex examples.

  • cargo run --bin hyper-warp-multiplex-server
  • cargo run --bin hyper-warp-multiplex-client

danny-osmos and others added 3 commits July 29, 2020 20:46
As per comment in PR #272

Resolves #269
Resolves #410
Route between http server and multiple grpc servers
@cthulhua
Copy link
Contributor

Thanks for taking this over; I’ve had a wild few months and haven’t been able to close this out

@olanod
Copy link

olanod commented Aug 14, 2020

I also ran into the need of having the gRPC server alongside a warp HTTP server on the same port(might even need websockets?) and wanted to know how feasible it is and if it would require work only on tonic's side.

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, left some nits on docs, otherwise I think we can merge this! Thanks!

tonic/src/transport/server/mod.rs Outdated Show resolved Hide resolved
tonic/src/transport/server/mod.rs Outdated Show resolved Hide resolved
Taking updates from PR feedback for RouterService & .into_service()
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants