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

Generated clients are missing routing headers #401

Closed
busunkim96 opened this issue Apr 22, 2020 · 6 comments · Fixed by #443 or #1133
Closed

Generated clients are missing routing headers #401

busunkim96 opened this issue Apr 22, 2020 · 6 comments · Fixed by #443 or #1133
Assignees
Labels
conversion blocking A surface delta, bug, missing feature, or other issue that prevents API conversion priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release.

Comments

@busunkim96
Copy link
Contributor

busunkim96 commented Apr 22, 2020

https://aip.dev/client-libraries/4222

It looks like the generator is not adding variables declared in the google.api.http annotation to routing headers.

https://github.com/googleapis/googleapis/blob/52701da10fec2a5f9796e8d12518c0fe574488fe/google/cloud/servicedirectory/v1beta1/registration_service.proto#L60-L65

See https://github.com/GoogleCloudPlatform/python-docs-samples/pull/3456/files#r412305714 for the modifications that need to be made to currently use the library.

Possibly related to #380.

Metadata should be added somewhere here (see this method for comparison))

@hkdevandla hkdevandla added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Apr 22, 2020
@software-dov
Copy link
Contributor

Just to clarify, is this an issue being seen with microgenerated client libraries?

@busunkim96
Copy link
Contributor Author

@rebeccawshaw
Copy link

Hi, is there any update on this bug? We need to get the python snippets in before our GA launch, as this is currently one of our blockers.

@busunkim96 busunkim96 added conversion blocking A surface delta, bug, missing feature, or other issue that prevents API conversion priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jun 4, 2020
@busunkim96
Copy link
Contributor Author

busunkim96 commented Jun 4, 2020

Bumping the priority on this as I'm seeing the same issue with KMS.

Errors look like:

EDIT: Looks like this is slightly different (perhaps related to pagination?). I don't see the error with my test project which has fewer resources.

E           grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
E               status = StatusCode.NOT_FOUND
E               details = "The request concerns location 'us-east1' but was sent to location 'global'. Either Cloud KMS is not available in 'us-east1' or the request was misrouted. Make sure the 'x-goog-request-params' metadata is set correctly; see https://cloud.google.com/kms/docs/grpc for more information."
E               debug_error_string = "{"created":"@1591243631.459760738","description":"Error received from peer ipv4:74.125.142.95:443","file":"src/core/lib/surface/call.cc","file_line":1056,"grpc_message":"The request concerns location 'us-east1' but was sent to location 'global'. Either Cloud KMS is not available in 'us-east1' or the request was misrouted. Make sure the 'x-goog-request-params' metadata is set correctly; see https://cloud.google.com/kms/docs/grpc for more information.","grpc_status":5}"
E           >

@busunkim96
Copy link
Contributor Author

The original issue with service_directory seems to be that the generator currently adds the metadata only for http.get. The methods missing metadata are of other types: patch, delete, post

  // Creates a namespace, and returns the new Namespace.
  rpc CreateNamespace(CreateNamespaceRequest) returns (Namespace) {
    option (google.api.http) = {
      post: "/v1beta1/{parent=projects/*/locations/*}/namespaces"
      body: "namespace"
    };
    option (google.api.method_signature) = "parent,namespace,namespace_id";
  }

@property
def field_headers(self) -> Sequence[str]:
"""Return the field headers defined for this method."""
http = self.options.Extensions[annotations_pb2.http]
if http.get:
return tuple(re.findall(r'\{([a-z][\w\d_.]+)=', http.get))
return ()

# Certain fields should be provided within the metadata header;
# add these here.
metadata = tuple(metadata) + (
gapic_v1.routing_header.to_grpc_metadata((
{%- for field_header in method.field_headers %}
('{{ field_header }}', request.{{ field_header }}),
{%- endfor %}
)),
)
{%- endif %}

@software-dov
Copy link
Contributor

Fixed by #443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conversion blocking A surface delta, bug, missing feature, or other issue that prevents API conversion priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release.
Projects
None yet
4 participants