-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
HttpBody mesage feature for stream RPC #1273
Conversation
@johanbrandhorst fuzzit and node_test failures are not related to changes. can you take a look at them ? thanks. |
@johanbrandhorst each stream header includes from http body now. please review. |
fuzzit and node_tests successful now. not sure what caused the issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work starting this! Could we also add a section to the HttpBody documentation that we support streams now?
Codecov Report
@@ Coverage Diff @@
## v2 #1273 +/- ##
==========================================
- Coverage 54.16% 53.95% -0.22%
==========================================
Files 42 42
Lines 4366 4383 +17
==========================================
Hits 2365 2365
- Misses 1745 1760 +15
- Partials 256 258 +2
Continue to review full report at Codecov.
|
can we take updating HTTPBody documentation as separate PR/MR ? can also share me where i can update the doc ? thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, but could you please add something to the httpbody docs? They are here: https://github.com/grpc-ecosystem/grpc-gateway/blob/v2/docs/_docs/httpbody.md.
Co-authored-by: Johan Brandhorst <[email protected]>
the httpbody.md is not updated as per v2. do you want me add stream example to the same one ? |
Yes please |
added example to httpbody readme. please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
It is same as #1268, but target branch is v2.