fix: add field headers for other http verbs #443
Conversation
googlebot
added
the
cla: yes
|
Ahh, it looks like the generated unit tests aren't getting 100% coverage. |
Codecov Report
@@ Coverage Diff @@
## master #443 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 26
Lines 1453 1453
Branches 300 300
=========================================
Hits 1453 1453
Continue to review full report at Codecov.
|
|
|
||
| # Mock the actual call within the gRPC stub, and fake the request. | ||
| with mock.patch.object( | ||
| type(client._transport.{{ method.name|snake_case }}), | ||
| '__call__') as call: | ||
| {% if method.void -%} |
There was a problem hiding this comment.
Copy-pasted from the unit test below this.
| ) | ||
| request = {{ method.input.ident }}() | ||
|
|
||
| {%- for field_header in method.field_headers %} |
There was a problem hiding this comment.
Tweaking this slightly for field headers like "/v1beta1/{endpoint.name=projects/*/locations/*/namespaces/*/services/*/endpoints/*}"
| @@ -333,7 +333,9 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): | |||
| metadata = tuple(metadata) + ( | |||
| gapic_v1.routing_header.to_grpc_metadata(( | |||
| {%- for field_header in method.field_headers %} | |||
| {%- if not method.client_streaming %} | |||
There was a problem hiding this comment.
Added this for a failing showcase test (see streaming method in proto and failing test).
Not sure if this is the right call though. What's the correct way to handle metadata/headers for streaming methods? @software-dov
There was a problem hiding this comment.
I was under the impression that headers for streaming methods don't make a lot of sense because they're not tightly bound to http semantics. If that's not correct, then I think we would need to ask someone who knows better.
| @@ -333,7 +333,9 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): | |||
| metadata = tuple(metadata) + ( | |||
| gapic_v1.routing_header.to_grpc_metadata(( | |||
| {%- for field_header in method.field_headers %} | |||
| {%- if not method.client_streaming %} | |||
There was a problem hiding this comment.
I was under the impression that headers for streaming methods don't make a lot of sense because they're not tightly bound to http semantics. If that's not correct, then I think we would need to ask someone who knows better.
Closes #401