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

documentation: on server, use FromIncomingContext for retrieving context and SetHeader, SetTrailer to send metadata to client #7238

Merged
merged 6 commits into from
Jun 4, 2024

Conversation

purnesh42H
Copy link
Contributor

@purnesh42H purnesh42H commented May 17, 2024

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 using OutgoingContext in server interceptor. Moreover, we should replace SendHeader to SetHeader in documentation as SendHeader is used to send the headers immediately to the client where as SetHeader is used to set headers that will be sent with the gRPC response, which is a more common use case.

RELEASE NOTES: None

@purnesh42H purnesh42H added the Type: Documentation Documentation or examples label May 17, 2024
@purnesh42H purnesh42H added this to the 1.65 Release milestone May 17, 2024
@purnesh42H purnesh42H assigned easwars and dfawley and unassigned easwars May 17, 2024
@easwars easwars assigned purnesh42H and unassigned easwars and dfawley May 20, 2024
@purnesh42H purnesh42H force-pushed the update-grpc-metadata-doc branch 5 times, most recently from 93f581a to 166c19c Compare May 21, 2024 04:12
@purnesh42H purnesh42H requested a review from easwars May 21, 2024 04:18
@purnesh42H purnesh42H assigned easwars and unassigned purnesh42H May 21, 2024
@easwars easwars assigned purnesh42H and unassigned easwars May 22, 2024
@easwars easwars assigned purnesh42H and unassigned easwars May 31, 2024
Copy link

codecov bot commented Jun 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.45%. Comparing base (adf976b) to head (0e6496c).
Report is 85 commits behind head on master.

Current head 0e6496c differs from pull request most recent head a13ca0c

Please upload reports for the commit a13ca0c to get more accurate results.

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     

see 48 files with indirect coverage changes

@purnesh42H purnesh42H changed the title documentation: Specify to use FromIncomingContext on server for retrieving context in grpc-metadata.md documentation: on server, use FromIncomingContext for retrieving context and SetHeader, SetTrailer to send metadata to client Jun 3, 2024
@purnesh42H purnesh42H force-pushed the update-grpc-metadata-doc branch 2 times, most recently from be97a6b to 6069210 Compare June 3, 2024 07:50
Comment on lines 85 to 88
header, err := c.Header()
if err != nil {
log.Fatalf("Receiving headers: %v", err)
} else {
Copy link
Contributor

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)
	}

Copy link
Contributor Author

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

Comment on lines 41 to 46
resp, err := client.UnaryEcho(
ctx,
&pb.EchoRequest{Message: "hello world"},
grpc.Header(&header),
grpc.Trailer(&trailer))

Copy link
Contributor

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)
	}

Copy link
Contributor Author

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
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@easwars easwars assigned purnesh42H and unassigned easwars Jun 3, 2024
@purnesh42H purnesh42H assigned easwars and unassigned purnesh42H Jun 4, 2024
@easwars easwars assigned purnesh42H and unassigned easwars Jun 4, 2024
@easwars easwars merged commit 6d23620 into grpc:master Jun 4, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Documentation or examples
Projects
None yet
4 participants