-
Notifications
You must be signed in to change notification settings - Fork 921
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
Support streaming services for JSON transcoding #5832
Comments
Note: At least to follow-up on #5829, we may want to consider returning a |
Hi @jrhee17 , I would like to contribute to this issue. Can you please assign it to me.? |
Sure, assigned |
Hi @jrhee17 , I followed the docs and tried to build Armeria locally using |
Hi @jrhee17, @ikhoon , I have successfully set up the dev environment in locally. I went through the Currently, two things I can see are mentioned, but I am not sure how to approach them yet. Can you please explain in brief or redirected me to some resource which might help me.
|
Currently once In order to support streaming client streaming rpc, we will likely need to
In order to support server streaming rpc, we likely need to do something similar to the response body. i.e. modify to convert each protobuf message to a json, and then send each newline delimited json. armeria/grpc/src/main/java/com/linecorp/armeria/server/grpc/HttpJsonTranscodingService.java Line 455 in 1e5cc28
Lastly, using reactive streams may not be performant so we may want to keep the current logic for unary and add a separate code path for streaming requests/responses. |
I missed this comment: #5832 (comment) You may specify |
First, thanks for your interest. However, I don't think this issue is good for the first contributor. This issue requires some experience with Armeria internals and gRPC specifications to implement. What do you think of finding other issues that are simpler and do not need to modify many files? It might be good to warm up and get familiar with the Armeria code base while working on an easy issue. |
How can I find easy or good to-start issues in armeria?? |
We've added a |
Currently, Armeria's
HttpJsonTranscodingService
doesn't support streaming messages and throws an exception.armeria/grpc/src/main/java/com/linecorp/armeria/server/grpc/HttpJsonTranscodingService.java
Lines 162 to 165 in 104855a
Recently, we saw an issue where some users with streaming services in their proto files weren't able to use transcoding at all since the above exception was thrown. #5828 #5829
Although a workaround was added to log a warning instead of throwing an exception, this also probably isn't ideal since users will still see warning logs for which they can't (and shouldn't) take action.
Moreover, it seems like
grpc-gateway
supports streaming as seen in the following issue where ndjson is used: grpc-ecosystem/grpc-gateway#581In an effort to 1) remove this discrepancy with upstream and 2) remove unnecessary warning logs, I suggest that we also support streaming messages for
HttpJsonTranscodingService
.The text was updated successfully, but these errors were encountered: