Skip to content

Conversation

@codyoss
Copy link
Member

@codyoss codyoss commented Dec 9, 2024

When a rpc returns an httpBody, see
https://pkg.go.dev/google.golang.org/genproto/googleapis/api/httpbody#HttpBody, we need to return a response from the helper so we can examine the headers.

When a rpc returns an httpBody, see
https://pkg.go.dev/google.golang.org/genproto/googleapis/api/httpbody#HttpBody,
we need to return a response from the helper so we can examine the headers.
@codyoss codyoss requested review from a team December 9, 2024 17:32
p("")
p(" buf, err := executeHTTPRequest(ctx, c.httpClient, httpReq, c.logger, %s, %q)", logBody, m.GetName())
if isHTTPBodyMessage {
p(" buf, httpRsp, err := executeHTTPRequestWithResponse(ctx, c.httpClient, httpReq, c.logger, %s, %q)", logBody, m.GetName())
Copy link
Member

Choose a reason for hiding this comment

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

How does httpRsp get used in the client?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would direct you to the updated generated file: internal/gengapic/testdata/rest_HttpBodyRPC.want

Copy link
Member

@quartzmo quartzmo Dec 9, 2024

Choose a reason for hiding this comment

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

Thanks, when I look at the whole file I see the usage:

if headers := httpRsp.Header; len(headers["Content-Type"]) > 0 {
			resp.ContentType = headers["Content-Type"][0]
		}

But it doesn't show up in the diff. Odd that this usage existed before.

@codyoss codyoss requested a review from quartzmo December 9, 2024 18:29
@codyoss codyoss merged commit b947c65 into googleapis:main Dec 9, 2024
7 checks passed
@codyoss codyoss deleted the body-resp-fix branch December 9, 2024 22:33
gcf-merge-on-green bot pushed a commit that referenced this pull request Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants