feat(web): Relax GrpcWebService request body type#2016
feat(web): Relax GrpcWebService request body type#2016tottoto merged 1 commit intohyperium:masterfrom
Conversation
| ResponseFuture { | ||
| case: Case::Other { | ||
| future: self.inner.call(req), | ||
| future: self.inner.call(req.map(tonic::body::boxed)), |
There was a problem hiding this comment.
Hmm, if we have to box the request no matter what, what are we gaining with this change?
There was a problem hiding this comment.
The benefit is the CorsGrpcWeb can handle any kind of body types which has the trait bounds.
There was a problem hiding this comment.
While the change itself seems fine, I'm still left with the thought: should we avoid all of the constraints that require a service that accepts Request<BoxBody>?
It seems like if we're going to bother to be totally generic over the request body type in GrpcWebService, we should also do so in CorsWebLayer? Maybe there's a very obvious reason why we can't, but looking at tower_http::cors::Cors shows that it's also generic over the request body... so it feels like we should be able to?
Since I assume your main question is whether we can avoid requiring a I guess so too. When I was working on this, I was also looking into the implementation of |
tottoto
force-pushed
the
tonic-web-body
branch
from
cfcbc7b to
4be6fc0
Compare
|
@tobz Are there any other concerns block this? If there are no problems with this, I will merge it at this stage. |
tobz
left a comment
tobz
left a comment
There was a problem hiding this comment.
Sorry for the delay here.
I do think it's fine for the moment and we can always relax the bounds further in the future.
tottoto
force-pushed
the
tonic-web-body
branch
from
4be6fc0 to
e5d6d5d
Compare
2 participants
Relaxes
GrpcWebService's request body type.