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

Add support for gRPC transcoding #122

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zZHorizonZz
Copy link

@zZHorizonZz zZHorizonZz commented Jan 22, 2025

Motivation:

This pull request introduces gRPC transcoding capabilities to the gRPC server. The most important changes include the addition of new interfaces for HTTP templates and variable bindings, as well as updates to existing classes to support these new features.

The code for path lookup and checking is based on code from grpc-transcoding-repo. I think, we shouldn't try to reinvent the wheel when a solution already exists. While there are some changes since the original code is in C++, the logic for matching and extracting HTTP requests remains largely similar. I have also converted several tests from the original repository for these classes.

Regarding the actual transcoding, I'm currently facing two issues. First, by definition, requests can have an empty body while the input gRPC message can still have fields. What I mean is that values can be passed in requests via path or query parameters. However, when I test this, the code gets stuck, my guess is that it's possibly waiting for a body. I need toinvestigate this further but would appreciate any help. The second issue is that in the generator, the code seems unable to read method options despite having them as dependencies. But, overall the transcoding seems to be working except for these two issues, though obviously, this PR will need additional work.

Also currently there isn't really support for streaming that would require websockets which is an entirely new thing in my opinion.

The code for the transcoding is reusing the wire format infrastructure by putting length and compression data before the json data, so that we don't need to create entirely new thing for this.

I appreciate any feedback and help with integrating this feature into the codebase.

Closes: #95

@zZHorizonZz
Copy link
Author

zZHorizonZz commented Jan 22, 2025

Well, I cannot really create an account on eclipse foundation as it's throwing error when I try to create one, I will wait a couple of days with that and see If it fixes itself or something.

@vietj
Copy link
Member

vietj commented Jan 22, 2025

I need to read more about it, however I think this feature should be implemented in its own module instead of being in grpc common

@zZHorizonZz
Copy link
Author

Yeah, I can do that. I have been wondering about that as well.

@zZHorizonZz zZHorizonZz force-pushed the grpc-web-it branch 2 times, most recently from 3f9b0d9 to e3002a9 Compare January 22, 2025 18:53
@vietj vietj added this to the 5.0.0 milestone Jan 23, 2025
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.

Grpc transcoding
2 participants