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

Support for gRPC Server Response Headers and Trailers, adding ClientAfter handler #479

Merged
merged 7 commits into from
Mar 5, 2017
Merged

Support for gRPC Server Response Headers and Trailers, adding ClientAfter handler #479

merged 7 commits into from
Mar 5, 2017

Conversation

basvanbeek
Copy link
Member

@basvanbeek basvanbeek commented Mar 3, 2017 •
edited
Loading

This is a (minor) breaking change to the existing gRPC transport but imho is a better and more Go kit idiomatic approach to handling server response metadata.

Stuff changed:

  • no more Server wide context as it purely uses the per request context as provided by gRPC
    -changed ServerResponseFunc to provide you with empty header and trailer metadata pointers so you can fill them with chained middlewares
  • new ClientResponseFunc to read the received header and trailer metadata
  • before and after functions in the client transport can now be added multiple times (fixes Allow multiple calls of http.ClientBefore() and http.ClientAfter() (for server as well) #477 too)

@basvanbeek basvanbeek mentioned this pull request Mar 3, 2017
Closed
@basvanbeek basvanbeek changed the title [WIP]: grpc ClientAfter handler and multiple calls to Client Before/After funcs. grpc ClientAfter handler and multiple calls to Client Before/After funcs. Mar 4, 2017
@basvanbeek basvanbeek changed the title grpc ClientAfter handler and multiple calls to Client Before/After funcs. WIP: grpc ClientAfter handler and multiple calls to Client Before/After funcs. Mar 5, 2017
@basvanbeek basvanbeek changed the title WIP: grpc ClientAfter handler and multiple calls to Client Before/After funcs. grpc ClientAfter handler and multiple calls to Client Before/After funcs. Mar 5, 2017
@basvanbeek basvanbeek changed the title grpc ClientAfter handler and multiple calls to Client Before/After funcs. Support for gRPC Server Response Headers and Trailers, adding ClientAfter handler Mar 5, 2017
peterbourgon
peterbourgon reviewed Mar 5, 2017
View reviewed changes
examples/addsvc/cmd/addsvc/main.go
@@ -222,7 +222,7 @@ func main() {
return
}

srv := addsvc.MakeGRPCServer(ctx, endpoints, tracer, logger)
srv := addsvc.MakeGRPCServer(endpoints, tracer, logger)
Copy link
Member

Choose a reason for hiding this comment

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

+1

@peterbourgon
Copy link
Member

Yep, this all scans perfectly, LGTM. In the merge commit, would you mind adding a small sentence or two about the changes required for consumers?

@basvanbeek
Copy link
Member Author

will do

@basvanbeek
Improved gRPC Request / Response Funcs
4079d84 
- Separated ClientRequestFunc and ServerRequestFunc to highlight that request metadata in a ServerRequestFunc is supposed to be immutable.
- Return context in ServerResponseFuncs like with the HTTP transport, to allow passing data between chained serverAfter middlewares and
finally to the encoding step so it can be used to alter the response payload prior to sending this response to the client.
@basvanbeek
updates to reflect change in ServerRequestFunc
8864ce8
@basvanbeek basvanbeek merged commit 322a9e0 into go-kit:master Mar 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterbourgon peterbourgon peterbourgon left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow multiple calls of http.ClientBefore() and http.ClientAfter() (for server as well)
2 participants
@basvanbeek @peterbourgon