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

Grpc transcoding #95

Open
zZHorizonZz opened this issue May 10, 2024 · 7 comments
Open

Grpc transcoding #95

zZHorizonZz opened this issue May 10, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@zZHorizonZz
Copy link

zZHorizonZz commented May 10, 2024

Describe the feature

This feature request proposes gRPC transcoding capabilities for the server. This would allow the server to accept json formatted grpc requests and transcode them according to the specifications defined in google.api.http.

Use cases

While grpc web is already implemented, it utilizes a unique format that differs from the transcoding of REST requests into grpc requests. Implementing this feature would be particularly useful when publicly exposing a grpc service, where supporting web based requests or providing users with a REST api option is desired.

Contribution

I am willing to contribute to the implementation of this feature. I have already implemented this in quarkus repository. After discussion in this PR, the maintainers concluded that this should be implemented directly in grpc server implementation.

@zZHorizonZz zZHorizonZz added the enhancement New feature or request label May 10, 2024
@vietj
Copy link
Member

vietj commented May 11, 2024

@zZHorizonZz we would be pleased to have this feature, perhaps we can discuss how the implementation should be crafted for the vertx-grpc project ?

@zZHorizonZz
Copy link
Author

zZHorizonZz commented May 11, 2024

@zZHorizonZz we would be pleased to have this feature, perhaps we can discuss how the implementation should be crafted for the vertx-grpc project ?

In my Quarkus PR, I needed to create custom GrpcReadStream and GrpcServerRequest implementations. This was necessary to merge path and query parameters into a JSON request. Also to find the appropriate marshallers, unmarshallers, etc, based on the current path, I needed to implement my own GrpcServer as well.

During debugging, I noticed that the code attempted to read a grpc prefix from the buffer, which doesn't exist in JSON requests. To fix this, I reimplemented several classes, including ReadStreamAdapter, WriteStreamAdapter, MessageDecoder, and MessageEncoder. In a previous conversation (quarkusio/quarkus#39991 (reply in thread)), it was mentioned that Quarkus uses Vert.x 4. After going trough the code, I found that in newer versions you have added grpc web, which can read JSON from requests. This should resolve the grpc prefix issue. I think?

However, there might still be problems in determining the correct request marshallers, unmarshallers based on its path and merging path and query parameters into the body.

@vietj
Copy link
Member

vietj commented May 11, 2024

we are planning to backport grpc-web to vertx 4, so we can assume developing your PR against master can be done and then we can backport everything to vertx 4.

@zZHorizonZz
Copy link
Author

zZHorizonZz commented May 12, 2024

Before I start implementing it, I will need to go through the code of grpc-web so that I understand it better, as I reckon the transcoding implementation will be using some elements from grpc-web.

@vietj
Copy link
Member

vietj commented May 13, 2024

sure @zZHorizonZz , there might be some decoupling / refactoring necessary because the current grpc-web integration uses inheritance with calls to super to override the behaviour for grpc-web sometimes

@vietj
Copy link
Member

vietj commented May 13, 2024

we can also schedule a call to discuss the plans if you are inclined

@zZHorizonZz
Copy link
Author

I wouldn't mind getting into a call, although I will be busy until the end of the month, so it would be in 2-3 weeks.

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

No branches or pull requests

2 participants