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

Impl tower::Service for tonic::transport::Channel #481

Closed
ajalab opened this issue Oct 24, 2020 · 1 comment · Fixed by #482
Closed

Impl tower::Service for tonic::transport::Channel #481

ajalab opened this issue Oct 24, 2020 · 1 comment · Fixed by #482
Milestone
0.5

Comments

@ajalab
Copy link
Contributor

ajalab commented Oct 24, 2020

Feature Request

Motivation

A type that implements tower::Service in the proper way implements GrpcService, but not the other way around.

Channel is a special implementation of GrpcService. It can potentially implement Service as well (making such a wrapper type is easy), but currently it doesn't. Due to this, we can't make use of various tower's wrapper services for a channel.

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let conn = tonic::transport::Endpoint::new("http://[::1]:50051")?
        .connect()
        .await?;
    // Wrap a channel with Timeout
    let conn = Timeout::new(conn, Duration::from_millis(1000));
    let mut client = GreeterClient::new(conn);  // Error!
    let request = tonic::Request::new(HelloRequest {
        name: "Tonic".into(),
    });

    let response = client.say_hello(request).await?;

    println!("RESPONSE={:?}", response);

    Ok(())
}
error[E0277]: the trait bound `tonic::transport::Channel: tower::Service<http::Request<tonic::body::BoxBody>>` is not satisfied
  --> src/client.rs:40:41
   |
40 |     let mut client = GreeterClient::new(conn);
   |                                         ^^^^ the trait `tower::Service<http::Request<tonic::body::BoxBody>>` is not implemented for `tonic::transport::Channel`
   |
   = note: required because of the requirements on the impl of `tower::Service<http::Request<tonic::body::BoxBody>>` for `tower::tower_timeout::Timeout<tonic::transport::Channel>`
   = note: required because of the requirements on the impl of `tonic::client::GrpcService<tonic::body::BoxBody>` for `tower::tower_timeout::Timeout<tonic::transport::Channel>`

Proposal

Let's implement tower::Service<http::Request<BoxBody>> for Channel, so that we can make use of tower service wrappers more easily.

Alternatives

To achieve the same goal, users may write a small wrapper for Channel which implements Service.

struct ChannelWrapper(Channel);

impl Service<http::Request<BoxBody>> for ChannelWrapper {
    type Response = http::Response<Body>;
    type Error = tonic::transport::Error;
    type Future = tonic::transport::channel::ResponseFuture;

    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.0.poll_ready(cx)
    }

    fn call(&mut self, req: http::Request<BoxBody>) -> Self::Future {
        let mut channel = self.0.clone();

        channel.call(req)
    }
}
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let conn = tonic::transport::Endpoint::new("http://[::1]:50051")?
        .connect()
        .await?;
    let conn = ChannelWrapper(conn);
    let conn = Timeout::new(conn, Duration::from_millis(1000));
    let mut client = GreeterClient::new(conn);
...

We can find a similar one in examples. But providing a canonical Service implementation for Channel would simplify user code?
ajalab added a commit to ajalab/tonic that referenced this issue Oct 24, 2020
@ajalab
transport: impl Service for Channel instead of GrpcService
3ae50d7 
This adds tower::Service impl for tonic::transport::Channel
and removes GrpcService impl declaration.

Channel still implements GrpcService, thanks to the general
impl declaration of GrpcService for types who implement Service.

Fixes hyperium#481.
@ajalab ajalab mentioned this issue Oct 24, 2020
Merged
@ajalab
Copy link
Contributor Author

ajalab commented Oct 24, 2020

I made one possible solution #482.

@akiradeveloper akiradeveloper mentioned this issue Nov 24, 2020
Closed
@LucioFranco LucioFranco added this to the 0.4 milestone Nov 27, 2020
LucioFranco pushed a commit that referenced this issue Jan 5, 2021
@ajalab
transport: impl Service for Channel instead of GrpcService (#482)
e7be352 
* transport: impl Service for Channel instead of GrpcService

This adds tower::Service impl for tonic::transport::Channel
and removes GrpcService impl declaration.

Channel still implements GrpcService, thanks to the general
impl declaration of GrpcService for types who implement Service.

Fixes #481.

* examples: stop using GrpcService to avoid ambiguity

* examples: add timeout example
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
0.5
Development

Successfully merging a pull request may close this issue.

transport: impl Service for Channel instead of GrpcService ajalab/tonic
2 participants
@ajalab @LucioFranco