Can we open up some of the codec methods?

[trezm]

Feature Request

I'd love for some of the methods within the codec module to be opened up to have pub instead of pub(crate) visibility.

Crates

tonic

Motivation

I've been toying around with adding some of the awesome work done here to my own projects, and I had to fork tonic to expose some of the methods within the codec module, as well as a few elsewhere.

Proposal

Move to pub permissions for:

• codec::decode::new_request
• codec::encode::encode_server
• codec::encode::EncodeBody
• request::from_http_parts
• request::into_http
• response::into_http

Alternatives

It would be nice if there was an easier way to plug into the hyper request -> proto and proto -> hyper response flows, but these were the fastest (read: not necessarily best,) changes to make to get to that end for the time being. Basically, it might be worth allowing Tonic to be used outside of Tower just over raw hyper requests and responses.

[LucioFranco]

@trezm hi! Could you explain more what your use case is? Mostly also curious why tower doesn't work for you as an abstraction?

[trezm]

You bet! I'm working on an alternative web framework (thruster) so it feels a little redundant to have an extra routing layer underneath.

I guess it would just be nice to have a really thin translation layer over the speed and ubiquity that Hyper provides out of the box -- or perhaps even a generalized layer of streams, but that's probably a better question for down the road.

As an example, here's what I currently have (with a bootleg version of Tonic with certain exports made public)

async fn decode_request<
  I: 'static + std::default::Default + prost::Message,
  O: 'static + std::default::Default + prost::Message
>(hyper_request: HyperRequest) -> I { // HyperRequest is just a wrapper around a Request from hyper
  let (parts, body) = hyper_request 
    .request
    .into_parts();

  let mut codec: tonic::codec::ProstCodec<O, I> =
    tonic::codec::ProstCodec::default();
  let mut stream =
    tonic::codec::Streaming::new_request(codec.decoder(), body);

  futures_util::pin_mut!(stream);

  let message = stream.try_next().await.unwrap().unwrap();
  let mut req: tonic::Request<I> = tonic::Request::from_http_parts(parts, message);

  req.into_inner()
}

[LucioFranco]

is there a reason you can't use https://docs.rs/tonic/0.1.0-alpha.6/tonic/client/struct.Grpc.html ?

[trezm]

Forgive me if this is a silly question, but doesn't that struct require a tower service? If that's the case the idea would be to give an option for people to use tonic without using tower as well, as it seems like going from stream to struct should be able to be decoupled from the tower portion of the project.

[LucioFranco]

@trezm Right, the goal of tonic is to abstract around tower and http-body. Is there a reason this doesn't work for you? or is it more so that its an extra api to learn?

The big reason why I didn't make those fn open is because I'm not sure if I want to stabalize them yet. I really want to expose a small surface area so that we can get tonic onto an actual stable release soon. I do plan on exposing them but just have had the lack of time right now to really look into them and ensure they are what I want. There is at least one change I have planned for it.

[trezm]

Ah, totally, not wanting to expose a potentially unstable api is a very reasonable response.

The reasoning here for me not particularly wanting to include tonic, is that it I'm working on an example integration for another web framework. Having two layered on top of each other for the sake of getting what's essentially a protocol feel redundant to me. It's not a matter of learning an extra api or because anything is wrong with tower, it's a great framework!

In my mind, if I were to incorporate it in its current form you'd get a data process that would look something like

thruster (routing) -> tonic (grpc) -> application code (handlers) -> tower (handlers)

and vice versa on the way back up, whereas ideally it would look more like

thruster (routing) -> tonic (grpc) -> application code (handlers)

This would follow the logical flow of data in the current stack much more closely. Does that make sense? If asking for a "stable-enough-to-be-public" api is too much at this moment, then I can figure something else out. That being said, I'm happy to start an RFC on the sort of backend-independent-layer that I'm describing!

[trezm]

@LucioFranco just pinging to see if you have any thoughts! Let me know if this is totally out of scope and I'll close the issue.

[LucioFranco]

@trezm hey! Sorry, I went on vacation since your last post so lost track of things 😄

I want to evaluate more what you are suggesting, I do in the end want to expose those codec fn but for now I'd like to keep them private. I want to push for a 0.1 release soon, once that is done lets pick this back up. Once, we have a version of tonic out that is 0.1 I am happy to look at your use case in thruster.

[trezm]

Howdy howdy! Just circling back with this now that there have been a few more releases. I'm happy to help in whatever capacity as well.

[LucioFranco]

@trezm hey! I've been quite busy so haven't had a chance to explore this yet. How about you hop on the discord and we can chat further there?

[trezm]

Sure! I'm a bit busy tonight, but I'll hop on and ping you in the next day or so!

…

On Thu, Apr 30, 2020 at 5:21 PM Lucio Franco ***@***.***> wrote: @trezm <https://github.com/trezm> hey! I've been quite busy so haven't had a chance to explore this yet. How about you hop on the discord and we can chat further there? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#133 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJWOLISH7AHBPWKKQWQDYLRPHTVZANCNFSM4JLPQJKA> .

[LucioFranco]

@trezm I think this has been solved, correct?

[trezm]

Yep -- still would be a "nice to have" but it's clear it's a non-goal of the project and there are definitely workarounds pointed out by @LucioFranco!! If anyone is interested in the end result, you can find an example here. Thanks again!