Skip to content

Conversation

@vchudnov-g
Copy link
Contributor

@vchudnov-g vchudnov-g commented Apr 26, 2024

Towards #1493, but note that this will occur for all Showcase services, not just Echo.

This can be verified manually on the shell via:

❯ curl -i -X POST -H "X-Goog-Api-Client: rest/0.0.0 gapic/0.0.0" -H 'Content-Type: application/json' http://localhost:7469/v1beta1/echo:echo -d'{"content":"g
reetings"}'                                                                                                                                                 
HTTP/1.1 200 OK
X-Showcase-Request-Accept: */*
X-Showcase-Request-Content-Length: 23
X-Showcase-Request-Content-Type: application/json
X-Showcase-Request-User-Agent: curl/8.5.0
X-Showcase-Request-X-Goog-Api-Client: rest/0.0.0 gapic/0.0.0
Date: Fri, 26 Apr 2024 21:33:24 GMT
Content-Length: 104
Content-Type: text/plain; charset=utf-8

{
  "content":  "greetings",
  "severity":  "UNNECESSARY",
  "requestId":  "",
  "otherRequestId":  ""
}

and works even for error responses:

❯ curl -i -X POST -H "X-Goog-Api-Client: rest/0.0.0 gapic/0.0.0" -H 'Content-Type: application/json' http://localhost:7469/v1beta1/echo:echo -d'{"foo":"greet
ings"}'                                                                                                                                                     
HTTP/1.1 400 Bad Request
X-Showcase-Request-Accept: */*
X-Showcase-Request-Content-Length: 19
X-Showcase-Request-Content-Type: application/json
X-Showcase-Request-User-Agent: curl/8.5.0
X-Showcase-Request-X-Goog-Api-Client: rest/0.0.0 gapic/0.0.0
Date: Fri, 26 Apr 2024 21:47:05 GMT
Content-Length: 160
Content-Type: text/plain; charset=utf-8

{"error":{"code":400,"message":"error reading body params '*': proto: (line 1:2): unknown field \"foo\"","details":null,"Body":"","Header":null,"Errors":null}

@vchudnov-g vchudnov-g requested review from noahdietz and parthea April 26, 2024 21:40
@vchudnov-g vchudnov-g requested review from a team as code owners April 26, 2024 21:40
@vchudnov-g vchudnov-g changed the title feat: echo request headers in response over REST transport feat(server): echo request headers in response over REST transport Apr 26, 2024
Copy link
Contributor

@parthea parthea left a comment

Choose a reason for hiding this comment

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

LGTM. I tested this locally using the code in googleapis/gapic-generator-python#1999

@parthea parthea merged commit de93c4c into main Apr 29, 2024
@parthea parthea deleted the echo-headers branch April 29, 2024 22:04
@noahdietz
Copy link
Collaborator

FWIW I don't think @vchudnov-g had landed on a header prefix yet, at least the suggestions in the document he wrote are different than what is implemented here...

@parthea
Copy link
Contributor

parthea commented Apr 29, 2024

At the time of merging this PR it seemed like we had settled on a prefix based on the comment thread in the document.

@noahdietz
Copy link
Collaborator

At the time of merging this PR it seemed like we had settled on a prefix based on the comment thread in the document.

You're right, @vchudnov-g had settled on that!

@vchudnov-g
Copy link
Contributor Author

One thought that occurs to me: maybe for the same reasons we have prefixes in the echoed http headers, we should also have them in the gRPC trailers that echo the gRPC request headers? Thoughts?

@noahdietz
Copy link
Collaborator

One thought that occurs to me: maybe for the same reasons we have prefixes in the echoed http headers, we should also have them in the gRPC trailers that echo the gRPC request headers? Thoughts?

Yeah it probably makes sense to do, but let's make sure we blast it out in release notes for those that already have trailer tests.

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.

3 participants