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

HttpBody mesage feature for stream RPC #1273

Merged
merged 5 commits into from
May 9, 2020

Conversation

adasari
Copy link
Contributor

@adasari adasari commented May 7, 2020

It is same as #1268, but target branch is v2.

@adasari
Copy link
Contributor Author

adasari commented May 7, 2020

@johanbrandhorst fuzzit and node_test failures are not related to changes. can you take a look at them ? thanks.

@adasari
Copy link
Contributor Author

adasari commented May 7, 2020

@johanbrandhorst each stream header includes from http body now. please review.
Also, can you elaborate #1268 (comment) ? i couldn't understand what you are referring to.

@adasari
Copy link
Contributor Author

adasari commented May 7, 2020

fuzzit and node_tests successful now. not sure what caused the issues.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a 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-io
Copy link

codecov-io commented May 8, 2020

Codecov Report

Merging #1273 into v2 will decrease coverage by 0.21%.
The diff coverage is 19.04%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
examples/internal/server/a_bit_of_everything.go 0.00% <0.00%> (ø)
runtime/handler.go 44.53% <50.00%> (-1.55%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e86fbdb...70b2e83. Read the comment docs.

@adasari
Copy link
Contributor Author

adasari commented May 8, 2020

can we take updating HTTPBody documentation as separate PR/MR ? can also share me where i can update the doc ? thanks.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a 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.

@adasari
Copy link
Contributor Author

adasari commented May 8, 2020

the httpbody.md is not updated as per v2. do you want me add stream example to the same one ?

@johanbrandhorst
Copy link
Collaborator

the httpbody.md is not updated as per v2. do you want me add stream example to the same one ?

Yes please

@adasari
Copy link
Contributor Author

adasari commented May 8, 2020

added example to httpbody readme. please review

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@johanbrandhorst johanbrandhorst merged commit dd7ce3e into grpc-ecosystem:v2 May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants