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 runtime.WithForwardResponseOption #53

Merged
merged 5 commits into from
Sep 28, 2015
Merged

Add runtime.WithForwardResponseOption #53

merged 5 commits into from
Sep 28, 2015

Conversation

bufdev
Copy link
Contributor

@bufdev bufdev commented Sep 15, 2015

This one may be a bit more work back and forth. The idea is that sometimes, you want to be able to modify values prior to sending the response over the wire. An example of this is https://go.pedge.io/dockervolume - I'd like to be able to modify the VolumeDriver interface to be a Protocol Buffers interface, and then use grpc-gateway to handle the API calls, but to do so, I need to be able to set a custom content type for API versioning with Docker.

This change should be backwards compatible. It also includes the changes from #52 so that the tests pass.

@bufdev
Copy link
Contributor Author

bufdev commented Sep 15, 2015

I'm not sure on the Travis build failure - advise?

@bufdev
Copy link
Contributor Author

bufdev commented Sep 15, 2015

Tests passing on travis now

@bufdev
Copy link
Contributor Author

bufdev commented Sep 24, 2015

Ping :)

Added http error code and error status for responseStreamChunk error
@bufdev
Copy link
Contributor Author

bufdev commented Sep 28, 2015

Hey @yugui, what do you think of this PR? I need this ASAP, can we pursue this?

@yugui
Copy link
Member

yugui commented Sep 28, 2015

@peter-edge sorry for being late. I'll take a look.

if _, err = w.Write(buf); err != nil {
glog.Errorf("Failed to write response: %v", err)
}
}

func handleForwardResponseOptions(ctx context.Context, w http.ResponseWriter, resp proto.Message, opts []func(context.Context, http.ResponseWriter, proto.Message) error) error {
if opts == nil || len(opts) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

opts == nil is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@yugui
Copy link
Member

yugui commented Sep 28, 2015

Could you resolve the merge conflict?

@bufdev
Copy link
Contributor Author

bufdev commented Sep 28, 2015

merge conflict fixed

yugui added a commit that referenced this pull request Sep 28, 2015
Add runtime.WithForwardResponseOption
@yugui yugui merged commit 537c715 into grpc-ecosystem:master Sep 28, 2015
ithinker1991 pushed a commit to tronprotocol/grpc-gateway that referenced this pull request May 27, 2018
…tNextMaintenanceTime_api

Feature/add get next maintenance time api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants