-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
documentation: on server, use FromIncomingContext for retrieving context and SetHeader
, SetTrailer
to send metadata to client
#7238
Conversation
93f581a
to
166c19c
Compare
166c19c
to
83b33ec
Compare
4099a1e
to
fb10717
Compare
379ba9b
to
3d2c2a5
Compare
3d2c2a5
to
9d08ab0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7238 +/- ##
==========================================
- Coverage 81.24% 80.45% -0.79%
==========================================
Files 345 348 +3
Lines 33941 33998 +57
==========================================
- Hits 27574 27354 -220
- Misses 5202 5449 +247
- Partials 1165 1195 +30 |
SetHeader
, SetTrailer
to send metadata to client
be97a6b
to
6069210
Compare
6069210
to
7a6355c
Compare
header, err := c.Header() | ||
if err != nil { | ||
log.Fatalf("Receiving headers: %v", err) | ||
} else { |
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.
The else
block can be eliminated and the block of code within it can be moved outside of the if
block.
header, err := c.Header()
if err != nil {
log.Fatalf("Receiving headers: %v", err)
}
fmt.Println("Received headers:")
for k, v := range header {
fmt.Printf("%s: %v\n", k, v)
}
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.
Good catch. I forgot to remove it
resp, err := client.UnaryEcho( | ||
ctx, | ||
&pb.EchoRequest{Message: "hello world"}, | ||
grpc.Header(&header), | ||
grpc.Trailer(&trailer)) | ||
|
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.
Go style does not impose line length restrictions on code lines, and it is recommended to not break-up code lines unless it significantly improves readability. See: https://google.github.io/styleguide/go/guide#line-length
You could simply write this as:
resp, err := client.UnaryEcho(ctx, &pb.EchoRequest{Message: "hello world"}, grpc.Header(&header), grpc.Trailer(&trailer))
if err != nil {
log.Fatalf("UnaryEcho: %v", err)
}
Or if you don't like such long lines, you could do:
opts := []grpc.CallOption{grpc.Header(&header), grpc.Trailer(&trailer))}
resp, err := client.UnaryEcho(ctx, &pb.EchoRequest{Message: "hello world"}, opts...)
if err != nil {
log.Fatalf("UnaryEcho: %v", err)
}
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.
Done
@@ -49,10 +49,22 @@ func unaryInterceptor(ctx context.Context, req any, info *grpc.UnaryServerInfo, | |||
return nil, errMissingMetadata | |||
} | |||
|
|||
// create and set metadata from interceptor to server |
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.
Comments that are complete sentences should be capitalized and punctuated like standard English sentences. Here and elsewhere in this PR.
See: https://google.github.io/styleguide/go/decisions#comment-sentences
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.
Done
0fb513c
to
a13ca0c
Compare
Fixes: #7215. It was opened while addressing the question #7125 in order to make it more clear for sending metadata from server to client. The sending metadata documentation section specifies it clearly in grpc-metadata.md to use
grpc.SetTrailer
However, we should make it more explicit to use
FromIncomingContext
on server as original issue was usingOutgoingContext
in server interceptor. Moreover, we should replaceSendHeader
toSetHeader
in documentation asSendHeader
is used to send the headers immediately to the client where asSetHeader
is used to set headers that will be sent with the gRPC response, which is a more common use case.RELEASE NOTES: None