From 8baf772261ae9cc0cded01f78cb3aad236b294f8 Mon Sep 17 00:00:00 2001 From: Luke Sneeringer Date: Wed, 20 Feb 2019 20:45:44 -0800 Subject: [PATCH] [refactor] New annotations (#92) This updates gapic-generator-python to use the current annotations. --- .circleci/config.yml | 10 ++--- Dockerfile | 4 +- gapic/schema/api.py | 4 +- gapic/schema/naming.py | 40 ++++++++++---------- gapic/schema/wrappers.py | 31 +++++++-------- gapic/templates/setup.py.j2 | 2 +- nox.py => noxfile.py | 2 +- setup.py | 2 +- tests/unit/generator/test_generator.py | 1 - tests/unit/schema/test_api.py | 8 ++-- tests/unit/schema/test_naming.py | 21 +++++----- tests/unit/schema/wrappers/test_field.py | 6 ++- tests/unit/schema/wrappers/test_method.py | 13 ++++--- tests/unit/schema/wrappers/test_service.py | 16 ++++---- tests/unit/schema/wrappers/test_signature.py | 10 ++--- 15 files changed, 81 insertions(+), 89 deletions(-) rename nox.py => noxfile.py (98%) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6a30a4915..0fb392bc0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -59,7 +59,7 @@ jobs: - run: name: Install nox and codecov. command: | - pip install --pre nox-automation + pip install nox pip install codecov - run: name: Run unit tests. @@ -81,7 +81,7 @@ jobs: - run: name: Install nox and codecov. command: | - pip install --pre nox-automation + pip install nox pip install codecov - run: name: Run unit tests. @@ -93,7 +93,7 @@ jobs: showcase: docker: - image: python:3.7-slim - - image: gcr.io/gapic-showcase/gapic-showcase:0.0.9 + - image: gcr.io/gapic-showcase/gapic-showcase:0.0.12 steps: - checkout - run: @@ -103,7 +103,7 @@ jobs: apt-get install -y curl pandoc unzip - run: name: Install nox. - command: pip install --pre nox-automation + command: pip install nox - run: name: Install protoc 3.6.1. command: | @@ -122,7 +122,7 @@ jobs: - checkout - run: name: Install nox. - command: pip install --pre nox-automation + command: pip install nox - run: name: Build the documentation. command: nox -s docs diff --git a/Dockerfile b/Dockerfile index 750c79a1c..b7a540010 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,8 +7,8 @@ RUN apt-get update \ && rm -rf /var/lib/apt/lists/* # Add protoc and our common protos. -COPY --from=gcr.io/gapic-images/api-common-protos:latest /usr/local/bin/protoc /usr/local/bin/protoc -COPY --from=gcr.io/gapic-images/api-common-protos:latest /protos/ /protos/ +COPY --from=gcr.io/gapic-images/api-common-protos:beta /usr/local/bin/protoc /usr/local/bin/protoc +COPY --from=gcr.io/gapic-images/api-common-protos:beta /protos/ /protos/ # Add our code to the Docker image. ADD . /usr/src/gapic-generator-python/ diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 0aaa67c85..7c8b7d367 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -23,7 +23,7 @@ from itertools import chain from typing import Callable, List, Mapping, Sequence, Set, Tuple -from google.api import annotations_pb2 +from google.longrunning import operations_pb2 from google.protobuf import descriptor_pb2 from gapic.schema import metadata @@ -525,7 +525,7 @@ def _get_methods(self, methods: List[descriptor_pb2.MethodDescriptorProto], # Iterate over the methods and collect them into a dictionary. answer = collections.OrderedDict() for meth_pb, i in zip(methods, range(0, sys.maxsize)): - lro = meth_pb.options.Extensions[annotations_pb2.operation] + lro = meth_pb.options.Extensions[operations_pb2.operation_info] # If the output type is google.longrunning.Operation, we use # a specialized object in its place. diff --git a/gapic/schema/naming.py b/gapic/schema/naming.py index 5bcfebe9f..f8d924edd 100644 --- a/gapic/schema/naming.py +++ b/gapic/schema/naming.py @@ -17,7 +17,7 @@ import re from typing import Iterable, Sequence, Tuple -from google.api import annotations_pb2 +from google.api import client_pb2 from google.protobuf import descriptor_pb2 from gapic import utils @@ -37,9 +37,12 @@ class Naming: namespace: Tuple[str] = dataclasses.field(default_factory=tuple) version: str = '' product_name: str = '' - product_url: str = '' proto_package: str = '' + def __post_init__(self): + if not self.product_name: + self.__dict__['product_name'] = self.name + @classmethod def build(cls, *file_descriptors: Iterable[descriptor_pb2.FileDescriptorProto] @@ -118,38 +121,33 @@ def build(cls, # # This creates a naming class non-empty metadata annotation and # uses Python's set logic to de-duplicate. There should only be one. - metadata_info = set() + explicit_pkgs = set() for fd in file_descriptors: - meta = fd.options.Extensions[annotations_pb2.metadata] + pkg = fd.options.Extensions[client_pb2.client_package] naming = cls( - name=meta.package_name or meta.product_name, - namespace=tuple(meta.package_namespace), - product_name=meta.product_name, - product_url=meta.product_uri, - version='', + name=pkg.title or pkg.product_title, + namespace=tuple(pkg.namespace), + version=pkg.version, ) if naming: - metadata_info.add(naming) + explicit_pkgs.add(naming) # Sanity check: Ensure that any google.api.metadata provisions were # consistent. - if len(metadata_info) > 1: + if len(explicit_pkgs) > 1: raise ValueError( - 'If the google.api.metadata annotation is provided in more ' - 'than one file, it must be consistent.', + 'If the google.api.client_package annotation is provided in ' + 'more than one file, it must be consistent.', ) # Merge the package naming information and the metadata naming # information, with the latter being preferred. # Return a Naming object which effectively merges them. - answer = package_info - if len(metadata_info): - for k, v in dataclasses.asdict(metadata_info.pop()).items(): - # Sanity check: We only want to overwrite anything if the - # new value is truthy. - if v: - answer = dataclasses.replace(answer, **{k: v}) - return answer + if len(explicit_pkgs): + return dataclasses.replace(package_info, + **dataclasses.asdict(explicit_pkgs.pop()), + ) + return package_info def __bool__(self): """Return True if any of the fields are truthy, False otherwise.""" diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 3da9d6cbe..177df3851 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -34,7 +34,8 @@ from typing import Iterable, List, Mapping, Sequence, Set, Tuple, Union from google.api import annotations_pb2 -from google.api import signature_pb2 +from google.api import client_pb2 +from google.api import field_behavior_pb2 from google.protobuf import descriptor_pb2 from gapic import utils @@ -92,7 +93,8 @@ def required(self) -> bool: Returns: bool: Whether this field is required. """ - return bool(self.options.Extensions[annotations_pb2.required]) + return (field_behavior_pb2.FieldBehavior.Value('REQUIRED') in + self.options.Extensions[field_behavior_pb2.field_behavior]) @utils.cached_property def type(self) -> Union['MessageType', 'EnumType', 'PythonType']: @@ -434,28 +436,22 @@ def ref_types(self) -> Sequence[Union[MessageType, EnumType]]: return tuple(answer) @utils.cached_property - def signatures(self) -> Tuple[signature_pb2.MethodSignature]: + def signatures(self) -> 'MethodSignatures': """Return the signature defined for this method.""" - sig_pb2 = self.options.Extensions[annotations_pb2.method_signature] - - # Sanity check: If there are no signatures (which should be by far - # the common case), just abort now. - if len(sig_pb2.fields) == 0: - return MethodSignatures(all=()) + signatures = self.options.Extensions[client_pb2.method_signature] # Signatures are annotated with an `additional_signatures` key that # allows for specifying additional signatures. This is an uncommon # case but we still want to deal with it. answer = [] - for sig in (sig_pb2,) + tuple(sig_pb2.additional_signatures): + for sig in signatures: # Build a MethodSignature object with the appropriate name # and fields. The fields are field objects, retrieved from # the method's `input` message. answer.append(MethodSignature( - name=sig.function_name if sig.function_name else self.name, fields=collections.OrderedDict([ (f.split('.')[-1], self.input.get_field(*f.split('.'))) - for f in sig.fields + for f in sig.split(',') ]), )) @@ -478,7 +474,6 @@ def with_context(self, *, collisions: Set[str]) -> 'Method': @dataclasses.dataclass(frozen=True) class MethodSignature: - name: str fields: Mapping[str, Field] @utils.cached_property @@ -551,8 +546,8 @@ def host(self) -> str: Returns: str: The hostname, with no protocol and no trailing ``/``. """ - if self.options.Extensions[annotations_pb2.default_host]: - return self.options.Extensions[annotations_pb2.default_host] + if self.options.Extensions[client_pb2.default_host]: + return self.options.Extensions[client_pb2.default_host] return None @property @@ -562,8 +557,10 @@ def oauth_scopes(self) -> Sequence[str]: Returns: Sequence[str]: A sequence of OAuth scopes. """ - oauth = self.options.Extensions[annotations_pb2.oauth] - return tuple(oauth.scopes) + # Return the OAuth scopes, split on comma. + return tuple([i.strip() for i in + self.options.Extensions[client_pb2.oauth_scopes].split(',') + if i]) @property def module_name(self) -> str: diff --git a/gapic/templates/setup.py.j2 b/gapic/templates/setup.py.j2 index 355ac6bfc..c1696fdb4 100644 --- a/gapic/templates/setup.py.j2 +++ b/gapic/templates/setup.py.j2 @@ -20,7 +20,7 @@ setuptools.setup( include_package_data=True, install_requires=( 'google-api-core >= 1.3.0, < 2.0.0dev', - 'googleapis-common-protos >= 1.6.0b6', + 'googleapis-common-protos >= 1.6.0b7', 'grpcio >= 1.10.0', 'proto-plus >= 0.3.0', ), diff --git a/nox.py b/noxfile.py similarity index 98% rename from nox.py rename to noxfile.py index 1e57c80f8..53ba3fb3c 100644 --- a/nox.py +++ b/noxfile.py @@ -56,7 +56,7 @@ def showcase(session): # Install a client library for Showcase. with tempfile.TemporaryDirectory() as tmp_dir: - showcase_version = '0.0.9' + showcase_version = '0.0.12' # Download the Showcase descriptor. session.run( diff --git a/setup.py b/setup.py index 66f95ab34..a87239368 100644 --- a/setup.py +++ b/setup.py @@ -42,7 +42,7 @@ include_package_data=True, install_requires=( 'click >= 6.7', - 'googleapis-common-protos >= 1.6.0b6', + 'googleapis-common-protos >= 1.6.0b7', 'jinja2 >= 2.10', 'protobuf >= 3.5.1', 'pypandoc >= 1.4', diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index ca16e8ea6..aef23eddd 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -260,5 +260,4 @@ def make_naming(**kwargs) -> naming.Naming: kwargs.setdefault('namespace', ('Google', 'Cloud')) kwargs.setdefault('version', 'v1') kwargs.setdefault('product_name', 'Hatstand') - kwargs.setdefault('product_url', 'https://cloud.google.com/hatstand/') return naming.Naming(**kwargs) diff --git a/tests/unit/schema/test_api.py b/tests/unit/schema/test_api.py index be2c1b80f..e1a355da3 100644 --- a/tests/unit/schema/test_api.py +++ b/tests/unit/schema/test_api.py @@ -17,8 +17,7 @@ import pytest -from google.api import annotations_pb2 -from google.api import longrunning_pb2 +from google.longrunning import operations_pb2 from google.protobuf import descriptor_pb2 from gapic.schema import api @@ -461,8 +460,8 @@ def test_lro(): input_type='google.example.v3.AsyncDoThingRequest', output_type='google.longrunning.Operation', ) - method_pb2.options.Extensions[annotations_pb2.operation].MergeFrom( - longrunning_pb2.OperationData( + method_pb2.options.Extensions[operations_pb2.operation_info].MergeFrom( + operations_pb2.OperationInfo( response_type='google.example.v3.AsyncDoThingResponse', metadata_type='google.example.v3.AsyncDoThingMetadata', ), @@ -606,5 +605,4 @@ def make_naming(**kwargs) -> naming.Naming: kwargs.setdefault('namespace', ('Google', 'Cloud')) kwargs.setdefault('version', 'v1') kwargs.setdefault('product_name', 'Hatstand') - kwargs.setdefault('product_url', 'https://cloud.google.com/hatstand/') return naming.Naming(**kwargs) diff --git a/tests/unit/schema/test_naming.py b/tests/unit/schema/test_naming.py index ad64c912e..f3ad0e09e 100644 --- a/tests/unit/schema/test_naming.py +++ b/tests/unit/schema/test_naming.py @@ -14,8 +14,7 @@ import pytest -from google.api import annotations_pb2 -from google.api import metadata_pb2 +from google.api import client_pb2 from google.protobuf import descriptor_pb2 from gapic.schema import naming @@ -116,8 +115,12 @@ def test_build_with_annotations(): name='spanner.proto', package='google.spanner.v1', ) - proto.options.Extensions[annotations_pb2.metadata].MergeFrom( - metadata_pb2.Metadata(package_namespace=['Google', 'Cloud']), + proto.options.Extensions[client_pb2.client_package].MergeFrom( + client_pb2.Package( + namespace=['Google', 'Cloud'], + title='Spanner', + version='v1', + ), ) n = naming.Naming.build(proto) assert n.name == 'Spanner' @@ -146,8 +149,8 @@ def test_inconsistent_metadata_error(): name='spanner.proto', package='google.spanner.v1', ) - proto1.options.Extensions[annotations_pb2.metadata].MergeFrom( - metadata_pb2.Metadata(package_namespace=['Google', 'Cloud']), + proto1.options.Extensions[client_pb2.client_package].MergeFrom( + client_pb2.Package(namespace=['Google', 'Cloud']), ) # Set up the second proto. @@ -156,9 +159,8 @@ def test_inconsistent_metadata_error(): name='spanner2.proto', package='google.spanner.v1', ) - proto2.options.Extensions[annotations_pb2.metadata].MergeFrom( - metadata_pb2.Metadata(package_namespace=['Google', 'Cloud'], - package_name='Spanner'), + proto2.options.Extensions[client_pb2.client_package].MergeFrom( + client_pb2.Package(title='Spanner', namespace=['Google', 'Cloud']), ) # This should error. Even though the data in the metadata is consistent, @@ -193,5 +195,4 @@ def make_naming(**kwargs) -> naming.Naming: kwargs.setdefault('namespace', ('Google', 'Cloud')) kwargs.setdefault('version', 'v1') kwargs.setdefault('product_name', 'Hatstand') - kwargs.setdefault('product_url', 'https://cloud.google.com/hatstand/') return naming.Naming(**kwargs) diff --git a/tests/unit/schema/wrappers/test_field.py b/tests/unit/schema/wrappers/test_field.py index 777450cfa..f60f64227 100644 --- a/tests/unit/schema/wrappers/test_field.py +++ b/tests/unit/schema/wrappers/test_field.py @@ -14,7 +14,7 @@ import pytest -from google.api import annotations_pb2 +from google.api import field_behavior_pb2 from google.protobuf import descriptor_pb2 from gapic.schema import wrappers @@ -83,7 +83,9 @@ def test_not_repeated(): def test_required(): field = make_field() - field.options.Extensions[annotations_pb2.required] = True + field.options.Extensions[field_behavior_pb2.field_behavior].append( + field_behavior_pb2.FieldBehavior.Value('REQUIRED') + ) assert field.required diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py index 7d8f983c5..5aa16a601 100644 --- a/tests/unit/schema/wrappers/test_method.py +++ b/tests/unit/schema/wrappers/test_method.py @@ -16,8 +16,8 @@ from typing import Sequence from google.api import annotations_pb2 +from google.api import client_pb2 from google.api import http_pb2 -from google.api import signature_pb2 from google.protobuf import descriptor_pb2 from gapic.schema import metadata @@ -47,9 +47,10 @@ def test_method_signature(): # Edit the underlying method pb2 post-hoc to add the appropriate annotation # (google.api.signature). - method.options.Extensions[annotations_pb2.method_signature].MergeFrom( - signature_pb2.MethodSignature(fields=['int_field', 'float_field']) - ) + method.options.Extensions[client_pb2.method_signature].append(','.join(( + 'int_field', + 'float_field', + ))) # We should get back just those two fields as part of the signature. assert len(method.signatures) == 1 @@ -73,8 +74,8 @@ def test_method_signature_nested(): # Edit the underlying method pb2 post-hoc to add the appropriate annotation # (google.api.signature). - method.options.Extensions[annotations_pb2.method_signature].MergeFrom( - signature_pb2.MethodSignature(fields=['inner.int_field']) + method.options.Extensions[client_pb2.method_signature].append( + 'inner.int_field', ) # We should get back just those two fields as part of the signature. diff --git a/tests/unit/schema/wrappers/test_service.py b/tests/unit/schema/wrappers/test_service.py index 158ef4b6a..3da253d0c 100644 --- a/tests/unit/schema/wrappers/test_service.py +++ b/tests/unit/schema/wrappers/test_service.py @@ -15,8 +15,8 @@ import typing from google.api import annotations_pb2 +from google.api import client_pb2 from google.api import http_pb2 -from google.api import signature_pb2 from google.protobuf import descriptor_pb2 from gapic.schema import imp @@ -103,7 +103,7 @@ def test_service_python_modules_signature(): type_name='a.b.c.v2.D', ), ), - method_signature=signature_pb2.MethodSignature(fields=['secs', 'd']), + method_signature='secs,d', ) # type=5 is int, so nothing is added. assert service.python_modules == ( @@ -137,8 +137,8 @@ def make_service(name: str = 'Placeholder', host: str = '', # appropriate. service_pb = descriptor_pb2.ServiceDescriptorProto(name=name) if host: - service_pb.options.Extensions[annotations_pb2.default_host] = host - service_pb.options.Extensions[annotations_pb2.oauth].scopes.extend(scopes) + service_pb.options.Extensions[client_pb2.default_host] = host + service_pb.options.Extensions[client_pb2.oauth_scopes] = ','.join(scopes) # Return a service object to test. return wrappers.Service( @@ -151,7 +151,7 @@ def make_service(name: str = 'Placeholder', host: str = '', # tests difficult to understand and maintain. def make_service_with_method_options(*, http_rule: http_pb2.HttpRule = None, - method_signature: signature_pb2.MethodSignature = None, + method_signature: str = '', in_fields: typing.Tuple[descriptor_pb2.FieldDescriptorProto] = () ) -> wrappers.Service: # Declare a method with options enabled for long-running operations and @@ -184,7 +184,7 @@ def get_method(name: str, lro_metadata_type: str = '', *, in_fields: typing.Tuple[descriptor_pb2.FieldDescriptorProto] = (), http_rule: http_pb2.HttpRule = None, - method_signature: signature_pb2.MethodSignature = None, + method_signature: str = '', ) -> wrappers.Method: input_ = get_message(in_type, fields=in_fields) output = get_message(out_type) @@ -204,8 +204,8 @@ def get_method(name: str, ext_key = annotations_pb2.http method_pb.options.Extensions[ext_key].MergeFrom(http_rule) if method_signature: - ext_key = annotations_pb2.method_signature - method_pb.options.Extensions[ext_key].MergeFrom(method_signature) + ext_key = client_pb2.method_signature + method_pb.options.Extensions[ext_key].append(method_signature) return wrappers.Method( method_pb=method_pb, diff --git a/tests/unit/schema/wrappers/test_signature.py b/tests/unit/schema/wrappers/test_signature.py index 0bb40affd..e9a22b0b6 100644 --- a/tests/unit/schema/wrappers/test_signature.py +++ b/tests/unit/schema/wrappers/test_signature.py @@ -25,7 +25,7 @@ def test_signature_dispatch_field(): ('foo', make_field(name='foo', type=T.Value('TYPE_INT32'))), ('bar', make_field(name='bar', type=T.Value('TYPE_BOOL'))), )) - signature = wrappers.MethodSignature(name='spam', fields=fields) + signature = wrappers.MethodSignature(fields=fields) assert signature.dispatch_field == fields['foo'] @@ -36,10 +36,8 @@ def test_signatures_magic_methods(): ('bar', make_field(name='bar', type=T.Value('TYPE_BOOL'))), )) signatures = wrappers.MethodSignatures(all=( - wrappers.MethodSignature(name='spam', fields=fields), - wrappers.MethodSignature(name='eggs', fields={ - 'foo': fields['foo'], - }), + wrappers.MethodSignature(fields=fields), + wrappers.MethodSignature(fields={'foo': fields['foo']}), )) assert len(signatures) == 2 assert tuple([i for i in signatures]) == signatures.all @@ -64,11 +62,9 @@ def test_signatures_single_dispatch(): ) signatures = wrappers.MethodSignatures(all=( wrappers.MethodSignature( - name='spam', fields=collections.OrderedDict(fields), ), wrappers.MethodSignature( - name='eggs', fields=collections.OrderedDict(reversed(fields)), ), ))