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.

Sign up for GitHub

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

Jump to bottom

http rule's response_body support to stream rpc #1208

Merged
merged 3 commits into from
Apr 17, 2020
Merged

http rule's response_body support to stream rpc #1208

merged 3 commits into from
Apr 17, 2020

Conversation

adasari
Copy link
Contributor

@adasari adasari commented Apr 13, 2020 •
edited
Loading

The changes regarding #1202
what is changed ?

  1. Added response body changes for runtime/handler.go and test cases
  2. template.go
  3. stream rpc with response body http rule field example and integration tests
  4. Existing test cases are failing in local due to port busy issue. changed the port of test cases so that it never fail.
  5. auto generated files from bazel sync
    Please review. thanks.

@adasari
Copy link
Contributor Author

adasari commented Apr 13, 2020 •
edited
Loading

@johanbrandhorst finally all passed. figured out managing branches of fork in hard way :). got delayed. Please take a look at now.

can we delete closed pull requests 1205 and 1206 ?

johanbrandhorst
johanbrandhorst requested changes Apr 14, 2020
View reviewed changes
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.

Thanks for making such a great start on this! I had some thoughts on the implementation, chiefly I'd like to keep the result envelope. Let me know what you think.

examples/internal/integration/integration_test.go Outdated Show resolved Hide resolved
examples/internal/integration/integration_test.go Outdated Show resolved Hide resolved
examples/internal/proto/examplepb/response_body_service.swagger.json Show resolved Hide resolved
examples/internal/server/responsebody.go Outdated Show resolved Hide resolved
runtime/handler.go Outdated Show resolved Hide resolved
runtime/handler_test.go Outdated Show resolved Hide resolved
johanbrandhorst
johanbrandhorst approved these changes Apr 17, 2020
View reviewed changes
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.

This looks great now, thanks for this contribution!

@johanbrandhorst johanbrandhorst merged commit a8e50a2 into grpc-ecosystem:master Apr 17, 2020
@johanbrandhorst johanbrandhorst mentioned this pull request Apr 17, 2020
Closed
pull bot pushed a commit to BuildingRobotics/grpc-gateway that referenced this pull request Apr 29, 2020
@adasari @pull
Add support for response_body in server-side streaming RPCs (grpc-eco…
3713f15 
…system#1208)

* stream changes

* review comment: incorrect port value in fataf message

* review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanbrandhorst johanbrandhorst johanbrandhorst approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adasari @johanbrandhorst @googlebot