From c1a6f891db8ce80a64d293ae8d8bed836b63a3c7 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Mon, 2 Nov 2020 17:49:42 +0000 Subject: [PATCH 01/14] feat: add rest transport generation for clients --- .../%namespace/%name/%version/__init__.py.j2 | 1 + .../%namespace/%name/__init__.py.j2 | 1 + gapic/schema/wrappers.py | 4 + .../%sub/services/%service/client.py.j2 | 2 + .../%service/transports/__init__.py.j2 | 3 + .../services/%service/transports/grpc.py.j2 | 2 +- .../services/%service/transports/rest.py.j2 | 200 ++++++++++++++++++ 7 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 diff --git a/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 b/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 index 749a408c42..ac517161fd 100644 --- a/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 +++ b/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 @@ -20,6 +20,7 @@ _lazy_type_to_package_map = { '{{ enum.name }}': '{{ enum.ident.package|join('.') }}.types.{{enum.ident.module }}', {%- endfor %} + {# TODO(yonmg): add rest transport service once I know what this is #} # Client classes and transports {%- for service in api.services.values() %} '{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}', diff --git a/gapic/ads-templates/%namespace/%name/__init__.py.j2 b/gapic/ads-templates/%namespace/%name/__init__.py.j2 index 749a408c42..ac517161fd 100644 --- a/gapic/ads-templates/%namespace/%name/__init__.py.j2 +++ b/gapic/ads-templates/%namespace/%name/__init__.py.j2 @@ -20,6 +20,7 @@ _lazy_type_to_package_map = { '{{ enum.name }}': '{{ enum.ident.package|join('.') }}.types.{{enum.ident.module }}', {%- endfor %} + {# TODO(yonmg): add rest transport service once I know what this is #} # Client classes and transports {%- for service in api.services.values() %} '{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}', diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index b5ae6fd0e9..7cebbe30e2 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -980,6 +980,10 @@ def grpc_transport_name(self): def grpc_asyncio_transport_name(self): return self.name + "GrpcAsyncIOTransport" + @property + def rest_transport_name(self): + return self.name + "RestTransport" + @property def has_lro(self) -> bool: """Return whether the service has a long-running method.""" diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index c3093aa1cf..60ea884727 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -32,6 +32,7 @@ from google.iam.v1 import policy_pb2 as policy # type: ignore from .transports.base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO from .transports.grpc import {{ service.grpc_transport_name }} from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }} +from .transports.rest import {{ service.rest_transport_name }} class {{ service.client_name }}Meta(type): @@ -44,6 +45,7 @@ class {{ service.client_name }}Meta(type): _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] _transport_registry['grpc'] = {{ service.grpc_transport_name }} _transport_registry['grpc_asyncio'] = {{ service.grpc_asyncio_transport_name }} + _transport_registry['rest'] = {{ service.name }}RestTransport def get_transport_class(cls, label: str = None, diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 index fa97f46164..8ff320d88a 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 @@ -7,17 +7,20 @@ from typing import Dict, Type from .base import {{ service.name }}Transport from .grpc import {{ service.name }}GrpcTransport from .grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport +from .rest import {{ service.name }}RestTransport # Compile a registry of transports. _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] _transport_registry['grpc'] = {{ service.name }}GrpcTransport _transport_registry['grpc_asyncio'] = {{ service.name }}GrpcAsyncIOTransport +_transport_registry['rest'] = {{ service.name }}RestTransport __all__ = ( '{{ service.name }}Transport', '{{ service.name }}GrpcTransport', '{{ service.name }}GrpcAsyncIOTransport', + '{{ service.name }}RestTransport', ) {% endblock %} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 index e2c68c483d..8be8de3b67 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 @@ -172,7 +172,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport): **kwargs) -> grpc.Channel: """Create and return a gRPC channel object. Args: - address (Optionsl[str]): The host for the channel to use. + address (Optional[str]): The host for the channel to use. credentials (Optional[~.Credentials]): The authorization credentials to attach to requests. These credentials identify this application to the service. If diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 new file mode 100644 index 0000000000..dbd1055335 --- /dev/null +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -0,0 +1,200 @@ +{% extends '_base.py.j2' %} + +{% block content %} +import warnings +from typing import Callable, Dict, Optional, Sequence, Tuple + +{% if service.has_lro %} +from google.api_core import operations_v1 +{%- endif %} +from google.api_core import gapic_v1 # type: ignore +from google import auth # type: ignore +from google.auth import credentials # type: ignore +from google.auth.transport.grpc import SslCredentials # type: ignore + +import grpc # type: ignore + +from google.auth.transport.requests import AuthorizedSession + +{# TODO: re-add python_import/ python_modules from removed diff/current grpc template code #} +{% filter sort_lines -%} +{% for method in service.methods.values() -%} +{{ method.input.ident.python_import }} +{{ method.output.ident.python_import }} +{% endfor -%} +{% if opts.add_iam_methods %} +from google.iam.v1 import iam_policy_pb2 as iam_policy # type: ignore +from google.iam.v1 import policy_pb2 as policy # type: ignore +{% endif %} +{% endfilter %} + +from .base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO + + +class {{ service.name }}RestTransport({{ service.name }}Transport): + """REST backend transport for {{ service.name }}. + + {{ service.meta.doc|rst(width=72, indent=4) }} + + This class defines the same methods as the primary client, so the + primary client can load the underlying transport implementation + and call it. + + It sends JSON representations of protocol buffers over HTTP/1.1 + """ + def __init__(self, *, + host: str = 'compute.googleapis.com', + credentials: credentials.Credentials = None, + credentials_file: str = None, + scopes: Sequence[str] = None, + ssl_channel_credentials: grpc.ChannelCredentials = None, + quota_project_id: Optional[str] = None, + client_info: gapic_v1.client_info.ClientInfo = DEFAULT_CLIENT_INFO, + ) -> None: + """Instantiate the transport. + + Args: + host ({% if service.host %}Optional[str]{% else %}str{% endif %}): + {{- ' ' }}The hostname to connect to. + credentials (Optional[google.auth.credentials.Credentials]): The + authorization credentials to attach to requests. These + credentials identify the application to the service; if none + are specified, the client will attempt to ascertain the + credentials from the environment. + + credentials_file (Optional[str]): A file with credentials that can + be loaded with :func:`google.auth.load_credentials_from_file`. + This argument is ignored if ``channel`` is provided. + scopes (Optional(Sequence[str])): A list of scopes. This argument is + ignored if ``channel`` is provided. + ssl_channel_credentials (grpc.ChannelCredentials): SSL credentials + for grpc channel. It is ignored if ``channel`` is provided. + quota_project_id (Optional[str]): An optional project to use for billing + and quota. + client_info (google.api_core.gapic_v1.client_info.ClientInfo): + The client info used to send a user-agent string along with + API requests. If ``None``, then default info will be used. + Generally, you only need to set this if you're developing + your own client library. + """ + super().__init__(host=host, credentials=credentials) + self._session = AuthorizedSession(self._credentials) + {%- if service.has_lro %} + + @property + def operations_client(self) -> operations_v1.OperationsClient: + """Create the client designed to process long-running operations. + + This property caches on the instance; repeated calls return the same + client. + """ + # Sanity check: Only create a new client if we do not already have one. + if 'operations_client' not in self.__dict__: + from google.api_core import grpc_helpers + self.__dict__['operations_client'] = operations_v1.OperationsClient( + grpc_helpers.create_channel( + self._host, + credentials=self._credentials, + scopes=self.AUTH_SCOPES, + ) + ) + + # Return the client from cache. + return self.__dict__['operations_client'] + {%- endif %} + + + {%- for method in service.methods.values() %} + {# TODO(yonmg): consider using enums #} + {%- set ns = namespace(http=None) %} + {%- for option in method.options.ListFields() %} + {%- if option[0].name == 'http' %} + {%- set ns.http = option[1] %} + {%- endif %} + {%- endfor %} + + {%- if ns.http %} + + + def {{ method.name|snake_case }}(self, + request: {{ method.input.ident }}, *, + metadata: Sequence[Tuple[str, str]] = (), + ) -> {{ method.output.ident }}: + r"""Call the {{- ' ' -}} + {{ (method.name|snake_case).replace('_',' ')|wrap( + width=70, offset=45, indent=8) }} + {{- ' ' -}} method over HTTP. + + Args: + request (~.{{ method.input.ident }}): + The request object. + {{ method.input.meta.doc|rst(width=72, indent=16) }} + metadata (Sequence[Tuple[str, str]]): Strings which should be + sent along with the request as metadata. + {%- if not method.void %} + + Returns: + ~.{{ method.output.ident }}: + {{ method.output.meta.doc|rst(width=72, indent=16) }} + {%- endif %} + """ + + # Check which http method to use + {%- if ns.http.get != '' %} + {%- set ns.httpMethod = {'method':'get','url':ns.http.get} %} + {%- elif ns.http.put != '' %} + {%- set ns.httpMethod = {'method':'put','url':ns.http.put} %} + {%- elif ns.http.post != '' %} + {%- set ns.httpMethod = {'method':'post','url':ns.http.post} %} + {%- elif ns.http.delete != '' %} + {%- set ns.httpMethod = {'method':'delete','url':ns.http.delete} %} + {%- elif ns.http.patch != '' %} + {%- set ns.httpMethod = {'method':'patch','url':ns.http.patch} %} + {%- elif ns.http.custom != '' %} + {%- set ns.httpMethod = {'method':'custom','url':ns.http.custom} %} + {%- else %} + {%- set ns.httpMethod = None %} + {%- endif %} + + {%- if ns.http.body != '' %} + # Jsonify the input + data = {{ method.output.ident }}.to_json( + {%- if ns.http.body == '*' %} + request + {%- else %} + request.body + {%- endif %} + ) + {%- endif %} + + {# TODO(yonmg): Write helper method for handling grpc transcoding url #} + # TODO(yonmg): need to handle grpc transcoding and parse url correctly + # current impl assumes simpler version of grpc transcoding + # Send the request + url = 'https://{host}{{ ns.httpMethod['url'] }}'.format( + host=self._host, + {%- for field in method.input.fields.keys() %} + {{ field }}=request.{{ field }}, + {%- endfor %} + ) + {% if not method.void %}response = {% endif %}self._session.{{ ns.httpMethod['method'] }}( + url, + {%- if ns.http.body != '' %} + json=data, + {%- endif %} + ) + {%- if not method.void %} + {%- endif %} + + # Return the response + return {{ method.output.ident }}.from_json(response.content) + {%- else %} + # No http annotation found for method {{ method.name|snake_case }}() + {%- endif %} + {%- endfor %} + + +__all__ = ( + '{{ service.name }}RestTransport', +) +{% endblock %} \ No newline at end of file From bd7c32dfd665ce2332898481b89f9c1c0f71d64d Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Mon, 2 Nov 2020 17:49:42 +0000 Subject: [PATCH 02/14] feat: add rest transport generation for clients --- .../%namespace/%name/%version/__init__.py.j2 | 1 + .../%namespace/%name/__init__.py.j2 | 1 + gapic/schema/wrappers.py | 4 + .../%sub/services/%service/client.py.j2 | 2 + .../%service/transports/__init__.py.j2 | 3 + .../services/%service/transports/grpc.py.j2 | 2 +- .../services/%service/transports/rest.py.j2 | 200 ++++++++++++++++++ tests/unit/schema/wrappers/test_service.py | 1 + 8 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 diff --git a/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 b/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 index 749a408c42..ac517161fd 100644 --- a/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 +++ b/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 @@ -20,6 +20,7 @@ _lazy_type_to_package_map = { '{{ enum.name }}': '{{ enum.ident.package|join('.') }}.types.{{enum.ident.module }}', {%- endfor %} + {# TODO(yonmg): add rest transport service once I know what this is #} # Client classes and transports {%- for service in api.services.values() %} '{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}', diff --git a/gapic/ads-templates/%namespace/%name/__init__.py.j2 b/gapic/ads-templates/%namespace/%name/__init__.py.j2 index 749a408c42..ac517161fd 100644 --- a/gapic/ads-templates/%namespace/%name/__init__.py.j2 +++ b/gapic/ads-templates/%namespace/%name/__init__.py.j2 @@ -20,6 +20,7 @@ _lazy_type_to_package_map = { '{{ enum.name }}': '{{ enum.ident.package|join('.') }}.types.{{enum.ident.module }}', {%- endfor %} + {# TODO(yonmg): add rest transport service once I know what this is #} # Client classes and transports {%- for service in api.services.values() %} '{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}', diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index b5ae6fd0e9..7cebbe30e2 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -980,6 +980,10 @@ def grpc_transport_name(self): def grpc_asyncio_transport_name(self): return self.name + "GrpcAsyncIOTransport" + @property + def rest_transport_name(self): + return self.name + "RestTransport" + @property def has_lro(self) -> bool: """Return whether the service has a long-running method.""" diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index c3093aa1cf..60ea884727 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -32,6 +32,7 @@ from google.iam.v1 import policy_pb2 as policy # type: ignore from .transports.base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO from .transports.grpc import {{ service.grpc_transport_name }} from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }} +from .transports.rest import {{ service.rest_transport_name }} class {{ service.client_name }}Meta(type): @@ -44,6 +45,7 @@ class {{ service.client_name }}Meta(type): _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] _transport_registry['grpc'] = {{ service.grpc_transport_name }} _transport_registry['grpc_asyncio'] = {{ service.grpc_asyncio_transport_name }} + _transport_registry['rest'] = {{ service.name }}RestTransport def get_transport_class(cls, label: str = None, diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 index fa97f46164..8ff320d88a 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 @@ -7,17 +7,20 @@ from typing import Dict, Type from .base import {{ service.name }}Transport from .grpc import {{ service.name }}GrpcTransport from .grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport +from .rest import {{ service.name }}RestTransport # Compile a registry of transports. _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] _transport_registry['grpc'] = {{ service.name }}GrpcTransport _transport_registry['grpc_asyncio'] = {{ service.name }}GrpcAsyncIOTransport +_transport_registry['rest'] = {{ service.name }}RestTransport __all__ = ( '{{ service.name }}Transport', '{{ service.name }}GrpcTransport', '{{ service.name }}GrpcAsyncIOTransport', + '{{ service.name }}RestTransport', ) {% endblock %} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 index e2c68c483d..8be8de3b67 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 @@ -172,7 +172,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport): **kwargs) -> grpc.Channel: """Create and return a gRPC channel object. Args: - address (Optionsl[str]): The host for the channel to use. + address (Optional[str]): The host for the channel to use. credentials (Optional[~.Credentials]): The authorization credentials to attach to requests. These credentials identify this application to the service. If diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 new file mode 100644 index 0000000000..dbd1055335 --- /dev/null +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -0,0 +1,200 @@ +{% extends '_base.py.j2' %} + +{% block content %} +import warnings +from typing import Callable, Dict, Optional, Sequence, Tuple + +{% if service.has_lro %} +from google.api_core import operations_v1 +{%- endif %} +from google.api_core import gapic_v1 # type: ignore +from google import auth # type: ignore +from google.auth import credentials # type: ignore +from google.auth.transport.grpc import SslCredentials # type: ignore + +import grpc # type: ignore + +from google.auth.transport.requests import AuthorizedSession + +{# TODO: re-add python_import/ python_modules from removed diff/current grpc template code #} +{% filter sort_lines -%} +{% for method in service.methods.values() -%} +{{ method.input.ident.python_import }} +{{ method.output.ident.python_import }} +{% endfor -%} +{% if opts.add_iam_methods %} +from google.iam.v1 import iam_policy_pb2 as iam_policy # type: ignore +from google.iam.v1 import policy_pb2 as policy # type: ignore +{% endif %} +{% endfilter %} + +from .base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO + + +class {{ service.name }}RestTransport({{ service.name }}Transport): + """REST backend transport for {{ service.name }}. + + {{ service.meta.doc|rst(width=72, indent=4) }} + + This class defines the same methods as the primary client, so the + primary client can load the underlying transport implementation + and call it. + + It sends JSON representations of protocol buffers over HTTP/1.1 + """ + def __init__(self, *, + host: str = 'compute.googleapis.com', + credentials: credentials.Credentials = None, + credentials_file: str = None, + scopes: Sequence[str] = None, + ssl_channel_credentials: grpc.ChannelCredentials = None, + quota_project_id: Optional[str] = None, + client_info: gapic_v1.client_info.ClientInfo = DEFAULT_CLIENT_INFO, + ) -> None: + """Instantiate the transport. + + Args: + host ({% if service.host %}Optional[str]{% else %}str{% endif %}): + {{- ' ' }}The hostname to connect to. + credentials (Optional[google.auth.credentials.Credentials]): The + authorization credentials to attach to requests. These + credentials identify the application to the service; if none + are specified, the client will attempt to ascertain the + credentials from the environment. + + credentials_file (Optional[str]): A file with credentials that can + be loaded with :func:`google.auth.load_credentials_from_file`. + This argument is ignored if ``channel`` is provided. + scopes (Optional(Sequence[str])): A list of scopes. This argument is + ignored if ``channel`` is provided. + ssl_channel_credentials (grpc.ChannelCredentials): SSL credentials + for grpc channel. It is ignored if ``channel`` is provided. + quota_project_id (Optional[str]): An optional project to use for billing + and quota. + client_info (google.api_core.gapic_v1.client_info.ClientInfo): + The client info used to send a user-agent string along with + API requests. If ``None``, then default info will be used. + Generally, you only need to set this if you're developing + your own client library. + """ + super().__init__(host=host, credentials=credentials) + self._session = AuthorizedSession(self._credentials) + {%- if service.has_lro %} + + @property + def operations_client(self) -> operations_v1.OperationsClient: + """Create the client designed to process long-running operations. + + This property caches on the instance; repeated calls return the same + client. + """ + # Sanity check: Only create a new client if we do not already have one. + if 'operations_client' not in self.__dict__: + from google.api_core import grpc_helpers + self.__dict__['operations_client'] = operations_v1.OperationsClient( + grpc_helpers.create_channel( + self._host, + credentials=self._credentials, + scopes=self.AUTH_SCOPES, + ) + ) + + # Return the client from cache. + return self.__dict__['operations_client'] + {%- endif %} + + + {%- for method in service.methods.values() %} + {# TODO(yonmg): consider using enums #} + {%- set ns = namespace(http=None) %} + {%- for option in method.options.ListFields() %} + {%- if option[0].name == 'http' %} + {%- set ns.http = option[1] %} + {%- endif %} + {%- endfor %} + + {%- if ns.http %} + + + def {{ method.name|snake_case }}(self, + request: {{ method.input.ident }}, *, + metadata: Sequence[Tuple[str, str]] = (), + ) -> {{ method.output.ident }}: + r"""Call the {{- ' ' -}} + {{ (method.name|snake_case).replace('_',' ')|wrap( + width=70, offset=45, indent=8) }} + {{- ' ' -}} method over HTTP. + + Args: + request (~.{{ method.input.ident }}): + The request object. + {{ method.input.meta.doc|rst(width=72, indent=16) }} + metadata (Sequence[Tuple[str, str]]): Strings which should be + sent along with the request as metadata. + {%- if not method.void %} + + Returns: + ~.{{ method.output.ident }}: + {{ method.output.meta.doc|rst(width=72, indent=16) }} + {%- endif %} + """ + + # Check which http method to use + {%- if ns.http.get != '' %} + {%- set ns.httpMethod = {'method':'get','url':ns.http.get} %} + {%- elif ns.http.put != '' %} + {%- set ns.httpMethod = {'method':'put','url':ns.http.put} %} + {%- elif ns.http.post != '' %} + {%- set ns.httpMethod = {'method':'post','url':ns.http.post} %} + {%- elif ns.http.delete != '' %} + {%- set ns.httpMethod = {'method':'delete','url':ns.http.delete} %} + {%- elif ns.http.patch != '' %} + {%- set ns.httpMethod = {'method':'patch','url':ns.http.patch} %} + {%- elif ns.http.custom != '' %} + {%- set ns.httpMethod = {'method':'custom','url':ns.http.custom} %} + {%- else %} + {%- set ns.httpMethod = None %} + {%- endif %} + + {%- if ns.http.body != '' %} + # Jsonify the input + data = {{ method.output.ident }}.to_json( + {%- if ns.http.body == '*' %} + request + {%- else %} + request.body + {%- endif %} + ) + {%- endif %} + + {# TODO(yonmg): Write helper method for handling grpc transcoding url #} + # TODO(yonmg): need to handle grpc transcoding and parse url correctly + # current impl assumes simpler version of grpc transcoding + # Send the request + url = 'https://{host}{{ ns.httpMethod['url'] }}'.format( + host=self._host, + {%- for field in method.input.fields.keys() %} + {{ field }}=request.{{ field }}, + {%- endfor %} + ) + {% if not method.void %}response = {% endif %}self._session.{{ ns.httpMethod['method'] }}( + url, + {%- if ns.http.body != '' %} + json=data, + {%- endif %} + ) + {%- if not method.void %} + {%- endif %} + + # Return the response + return {{ method.output.ident }}.from_json(response.content) + {%- else %} + # No http annotation found for method {{ method.name|snake_case }}() + {%- endif %} + {%- endfor %} + + +__all__ = ( + '{{ service.name }}RestTransport', +) +{% endblock %} \ No newline at end of file diff --git a/tests/unit/schema/wrappers/test_service.py b/tests/unit/schema/wrappers/test_service.py index c4c8d9b838..ef14e27a6f 100644 --- a/tests/unit/schema/wrappers/test_service.py +++ b/tests/unit/schema/wrappers/test_service.py @@ -59,6 +59,7 @@ def test_service_properties(): assert service.transport_name == 'ThingDoerTransport' assert service.grpc_transport_name == 'ThingDoerGrpcTransport' assert service.grpc_asyncio_transport_name == 'ThingDoerGrpcAsyncIOTransport' + assert service.rest_transport_name == 'ThingDoerRestTransport' def test_service_host(): From b41db3136f5fd4499fccf1ff7ec9c8efc4ba5395 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Fri, 6 Nov 2020 19:42:25 +0000 Subject: [PATCH 03/14] feat: add transport flag --- gapic/generator/generator.py | 4 ++++ .../%sub/services/%service/async_client.py.j2 | 1 + .../%sub/services/%service/client.py.j2 | 18 +++++++++++++----- .../%service/transports/__init__.py.j2 | 14 +++++++++++++- gapic/utils/options.py | 9 +++++++-- 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/gapic/generator/generator.py b/gapic/generator/generator.py index 5bd8a4f3c9..92e8571eae 100644 --- a/gapic/generator/generator.py +++ b/gapic/generator/generator.py @@ -274,6 +274,10 @@ def _render_template( if ( skip_subpackages and service.meta.address.subpackage != api_schema.subpackage_view + or + 'grpc' in template_name and 'grpc' not in opts.transport + or + 'rest' in template_name and 'rest' not in opts.transport ): continue diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 index 9d5150d869..d851dec06f 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 @@ -30,6 +30,7 @@ from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }} from .client import {{ service.client_name }} +{# TODO(yonmg): handle rest transport async client interaction #} class {{ service.async_client_name }}: """{{ service.meta.doc|rst(width=72, indent=4) }}""" diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index 60ea884727..5c58fdf5f7 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -30,9 +30,13 @@ from google.iam.v1 import policy_pb2 as policy # type: ignore {% endif %} {% endfilter %} from .transports.base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO -from .transports.grpc import {{ service.grpc_transport_name }} -from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }} -from .transports.rest import {{ service.rest_transport_name }} +{%- if 'grpc' in opts.transport %} +from .grpc import {{ service.name }}GrpcTransport +from .grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport +{%- endif %} +{%- if 'rest' in opts.transport %} +from .rest import {{ service.name }}RestTransport +{%- endif %} class {{ service.client_name }}Meta(type): @@ -43,9 +47,13 @@ class {{ service.client_name }}Meta(type): objects. """ _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] - _transport_registry['grpc'] = {{ service.grpc_transport_name }} - _transport_registry['grpc_asyncio'] = {{ service.grpc_asyncio_transport_name }} + {%- if 'grpc' in opts.transport %} + _transport_registry['grpc'] = {{ service.name }}GrpcTransport + _transport_registry['grpc_asyncio'] = {{ service.name }}GrpcAsyncIOTransport + {%- endif %} + {%- if 'rest' in opts.transport %} _transport_registry['rest'] = {{ service.name }}RestTransport + {%- endif %} def get_transport_class(cls, label: str = None, diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 index 8ff320d88a..bd7981387f 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 @@ -5,22 +5,34 @@ from collections import OrderedDict from typing import Dict, Type from .base import {{ service.name }}Transport +{%- if 'grpc' in opts.transport %} from .grpc import {{ service.name }}GrpcTransport from .grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport +{%- endif %} +{%- if 'rest' in opts.transport %} from .rest import {{ service.name }}RestTransport +{%- endif %} + # Compile a registry of transports. _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] +{%- if 'grpc' in opts.transport %} _transport_registry['grpc'] = {{ service.name }}GrpcTransport _transport_registry['grpc_asyncio'] = {{ service.name }}GrpcAsyncIOTransport +{%- endif %} +{%- if 'rest' in opts.transport %} _transport_registry['rest'] = {{ service.name }}RestTransport - +{%- endif %} __all__ = ( '{{ service.name }}Transport', + {%- if 'grpc' in opts.transport %} '{{ service.name }}GrpcTransport', '{{ service.name }}GrpcAsyncIOTransport', + {%- endif %} + {%- if 'rest' in opts.transport %} '{{ service.name }}RestTransport', + {%- endif %} ) {% endblock %} diff --git a/gapic/utils/options.py b/gapic/utils/options.py index c3a1ef322e..21f059bede 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -14,7 +14,7 @@ from collections import defaultdict from os import path -from typing import Any, DefaultDict, Dict, FrozenSet, List, Optional, Tuple +from typing import Any, DefaultDict, Dict, FrozenSet, List, Optional, Tuple, Set import dataclasses import json @@ -40,6 +40,8 @@ class Options: lazy_import: bool = False old_naming: bool = False add_iam_methods: bool = False + # TODO(yonmg): should there be an enum for transport type? + transport: List[str] = None # Class constants PYTHON_GAPIC_PREFIX: str = 'python-gapic-' @@ -49,6 +51,7 @@ class Options: 'samples', # output dir 'lazy-import', # requires >= 3.7 'add-iam-methods', # microgenerator implementation for `reroute_to_grpc_interface` + 'transport', # transport type (i.e. grpc, rest, custom.[something], etc?) )) @classmethod @@ -86,7 +89,7 @@ def build(cls, opt_string: str) -> 'Options': # Throw away other options not meant for us. if not opt.startswith(cls.PYTHON_GAPIC_PREFIX): continue - + # Set the option, using a key with the "python-gapic-" prefix # stripped. # @@ -121,6 +124,7 @@ def tweak_path(p): # Build the options instance. sample_paths = opts.pop('samples', []) + answer = Options( name=opts.pop('name', ['']).pop(), namespace=tuple(opts.pop('namespace', [])), @@ -134,6 +138,7 @@ def tweak_path(p): lazy_import=bool(opts.pop('lazy-import', False)), old_naming=bool(opts.pop('old-naming', False)), add_iam_methods=bool(opts.pop('add-iam-methods', False)), + transport=opts.pop('transport', ['grpc'])[0].split('+') ) # Note: if we ever need to recursively check directories for sample From ce69e7de76543f74f5cd2d2b67d2623c33e8c2ff Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Fri, 6 Nov 2020 19:44:42 +0000 Subject: [PATCH 04/14] refactor: moved template logic outside --- gapic/cli/generate.py | 1 - gapic/schema/wrappers.py | 23 ++++++++ .../services/%service/transports/grpc.py.j2 | 7 ++- .../%service/transports/grpc_asyncio.py.j2 | 7 ++- .../services/%service/transports/rest.py.j2 | 57 ++++++------------- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/gapic/cli/generate.py b/gapic/cli/generate.py index b0e50ccf2e..7950e462ff 100644 --- a/gapic/cli/generate.py +++ b/gapic/cli/generate.py @@ -42,7 +42,6 @@ def generate( # Pull apart arguments in the request. opts = Options.build(req.parameter) - # Determine the appropriate package. # This generator uses a slightly different mechanism for determining # which files to generate; it tracks at package level rather than file diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 7cebbe30e2..1da269c9ce 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -719,6 +719,7 @@ def _client_output(self, enable_asyncio: bool): # Return the usual output. return self.output + # TODO(yonmg): remove or rewrite. don't think it performs as intended @property def field_headers(self) -> Sequence[str]: """Return the field headers defined for this method.""" @@ -736,7 +737,28 @@ def field_headers(self) -> Sequence[str]: ] return next((tuple(pattern.findall(verb)) for verb in potential_verbs if verb), ()) + + @property + def http_opt(self) -> Dict[str, str]: + """Return the http option for this method.""" + http = self.options.Extensions[annotations_pb2.http].ListFields() + if len(http) < 1: + return None + + http_method = http[0] + answer: Dict[str, str] = { + 'method':http_method[0].name, + 'url':http_method[1], + } + if len(http) > 1: + http_opt = http[1] + answer[http_opt[0].name] = http_opt[1] + + # TODO(yonmg): handle nested fields & fields past body i.e. 'additional bindings' + # TODO(yonmg): enums for http verbs? + return answer + # TODO(yonmg): refactor as there may be more than one method signature @utils.cached_property def flattened_fields(self) -> Mapping[str, Field]: """Return the signature defined for this method.""" @@ -786,6 +808,7 @@ def grpc_stub_type(self) -> str: server='stream' if self.server_streaming else 'unary', ) + # TODO(yonmg): figure out why idempotent is reliant on http annotation @utils.cached_property def idempotent(self) -> bool: """Return True if we know this method is idempotent, False otherwise. diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 index 8be8de3b67..c7bd0cfc1e 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 @@ -151,6 +151,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport): ) self._stubs = {} # type: Dict[str, Callable] + {%- if service.has_lro %} self._operations_client = None {%- endif %} # Run the base constructor. super().__init__( @@ -220,13 +221,13 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport): client. """ # Sanity check: Only create a new client if we do not already have one. - if 'operations_client' not in self.__dict__: - self.__dict__['operations_client'] = operations_v1.OperationsClient( + if self._operations_client is None: + self._operations_client = operations_v1.OperationsClient( self.grpc_channel ) # Return the client from cache. - return self.__dict__['operations_client'] + return self._operations_client {%- endif %} {%- for method in service.methods.values() %} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 index 6399f1f1cd..d2b162d4e3 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 @@ -205,6 +205,7 @@ class {{ service.grpc_asyncio_transport_name }}({{ service.name }}Transport): ) self._stubs = {} + {%- if service.has_lro %} self._operations_client = None {%- endif %} @property def grpc_channel(self) -> aio.Channel: @@ -225,13 +226,13 @@ class {{ service.grpc_asyncio_transport_name }}({{ service.name }}Transport): client. """ # Sanity check: Only create a new client if we do not already have one. - if 'operations_client' not in self.__dict__: - self.__dict__['operations_client'] = operations_v1.OperationsAsyncClient( + if self._operations_client is None: + self._operations_client = operations_v1.OperationsAsyncClient( self.grpc_channel ) # Return the client from cache. - return self.__dict__['operations_client'] + return self._operations_client {%- endif %} {%- for method in service.methods.values() %} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index dbd1055335..a5d9c5dd34 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -42,8 +42,9 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): It sends JSON representations of protocol buffers over HTTP/1.1 """ + {# TODO(yonmg): handle mtls stuff if that's relevant for rest transport #} def __init__(self, *, - host: str = 'compute.googleapis.com', + host: str{% if service.host %} = '{{ service.host }}'{% endif %}, credentials: credentials.Credentials = None, credentials_file: str = None, scopes: Sequence[str] = None, @@ -79,8 +80,12 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): """ super().__init__(host=host, credentials=credentials) self._session = AuthorizedSession(self._credentials) - {%- if service.has_lro %} + {%- if service.has_lro %} + self._operations_client = None + {%- endif %} + {%- if service.has_lro %} + @property def operations_client(self) -> operations_v1.OperationsClient: """Create the client designed to process long-running operations. @@ -89,9 +94,9 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): client. """ # Sanity check: Only create a new client if we do not already have one. - if 'operations_client' not in self.__dict__: + if self._operations_client is None: from google.api_core import grpc_helpers - self.__dict__['operations_client'] = operations_v1.OperationsClient( + self._operations_client = operations_v1.OperationsClient( grpc_helpers.create_channel( self._host, credentials=self._credentials, @@ -100,22 +105,12 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): ) # Return the client from cache. - return self.__dict__['operations_client'] + return self._operations_client {%- endif %} {%- for method in service.methods.values() %} - {# TODO(yonmg): consider using enums #} - {%- set ns = namespace(http=None) %} - {%- for option in method.options.ListFields() %} - {%- if option[0].name == 'http' %} - {%- set ns.http = option[1] %} - {%- endif %} - {%- endfor %} - - {%- if ns.http %} - - + {%- if method.http_opt %} def {{ method.name|snake_case }}(self, request: {{ method.input.ident }}, *, metadata: Sequence[Tuple[str, str]] = (), @@ -139,27 +134,10 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): {%- endif %} """ - # Check which http method to use - {%- if ns.http.get != '' %} - {%- set ns.httpMethod = {'method':'get','url':ns.http.get} %} - {%- elif ns.http.put != '' %} - {%- set ns.httpMethod = {'method':'put','url':ns.http.put} %} - {%- elif ns.http.post != '' %} - {%- set ns.httpMethod = {'method':'post','url':ns.http.post} %} - {%- elif ns.http.delete != '' %} - {%- set ns.httpMethod = {'method':'delete','url':ns.http.delete} %} - {%- elif ns.http.patch != '' %} - {%- set ns.httpMethod = {'method':'patch','url':ns.http.patch} %} - {%- elif ns.http.custom != '' %} - {%- set ns.httpMethod = {'method':'custom','url':ns.http.custom} %} - {%- else %} - {%- set ns.httpMethod = None %} - {%- endif %} - - {%- if ns.http.body != '' %} + {%- if 'body' in method.http_opt.keys() %} # Jsonify the input data = {{ method.output.ident }}.to_json( - {%- if ns.http.body == '*' %} + {%- if method.http_opt['body'] == '*' %} request {%- else %} request.body @@ -171,15 +149,15 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): # TODO(yonmg): need to handle grpc transcoding and parse url correctly # current impl assumes simpler version of grpc transcoding # Send the request - url = 'https://{host}{{ ns.httpMethod['url'] }}'.format( + url = 'https://{host}{{ method.http_opt['url'] }}'.format( host=self._host, {%- for field in method.input.fields.keys() %} {{ field }}=request.{{ field }}, {%- endfor %} ) - {% if not method.void %}response = {% endif %}self._session.{{ ns.httpMethod['method'] }}( + {% if not method.void %}response = {% endif %}self._session.{{ method.http_opt['method'] }}( url, - {%- if ns.http.body != '' %} + {%- if 'body' in method.http_opt.keys() %} json=data, {%- endif %} ) @@ -189,8 +167,7 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): # Return the response return {{ method.output.ident }}.from_json(response.content) {%- else %} - # No http annotation found for method {{ method.name|snake_case }}() - {%- endif %} + {%- endif %} {%- endfor %} From 89fa08b3032acb5de8d806bbba0876f92561de1c Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Mon, 9 Nov 2020 20:19:40 +0000 Subject: [PATCH 05/14] fix: small fixes in transport option logic --- gapic/generator/generator.py | 13 ++++++++++--- gapic/schema/wrappers.py | 9 +++++---- .../%sub/services/%service/transports/rest.py.j2 | 6 ++---- gapic/utils/options.py | 5 +++-- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/gapic/generator/generator.py b/gapic/generator/generator.py index 92e8571eae..6460c3b182 100644 --- a/gapic/generator/generator.py +++ b/gapic/generator/generator.py @@ -275,9 +275,8 @@ def _render_template( skip_subpackages and service.meta.address.subpackage != api_schema.subpackage_view or - 'grpc' in template_name and 'grpc' not in opts.transport - or - 'rest' in template_name and 'rest' not in opts.transport + 'transport' in template_name + and not self._is_desired_transport(template_name, opts) ): continue @@ -297,6 +296,14 @@ def _render_template( template_name, api_schema=api_schema, opts=opts)) return answer + def _is_desired_transport(self, template_name: str, opts: Options) -> bool: + """Returns true if template name contains a desired transport""" + desired_transports = ['__init__', 'base'] + opts.transport + for transport in desired_transports: + if transport in template_name: + return True + return False + def _get_file( self, template_name: str, diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 1da269c9ce..6b91a2cc40 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -742,13 +742,14 @@ def field_headers(self) -> Sequence[str]: def http_opt(self) -> Dict[str, str]: """Return the http option for this method.""" http = self.options.Extensions[annotations_pb2.http].ListFields() + answer: Dict[str, str] = None if len(http) < 1: - return None + return answer http_method = http[0] - answer: Dict[str, str] = { - 'method':http_method[0].name, - 'url':http_method[1], + answer = { + 'method': http_method[0].name, + 'url': http_method[1], } if len(http) > 1: http_opt = http[1] diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index a5d9c5dd34..913f6a774e 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -107,10 +107,9 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): # Return the client from cache. return self._operations_client {%- endif %} - - {%- for method in service.methods.values() %} {%- if method.http_opt %} + def {{ method.name|snake_case }}(self, request: {{ method.input.ident }}, *, metadata: Sequence[Tuple[str, str]] = (), @@ -162,11 +161,10 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): {%- endif %} ) {%- if not method.void %} - {%- endif %} # Return the response return {{ method.output.ident }}.from_json(response.content) - {%- else %} + {%- endif %} {%- endif %} {%- endfor %} diff --git a/gapic/utils/options.py b/gapic/utils/options.py index 21f059bede..df18c35619 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -40,8 +40,8 @@ class Options: lazy_import: bool = False old_naming: bool = False add_iam_methods: bool = False - # TODO(yonmg): should there be an enum for transport type? - transport: List[str] = None + # TODO(yon-mg): should there be an enum for transport type? + transport: List[str] = dataclasses.field(default_factory=lambda: []) # Class constants PYTHON_GAPIC_PREFIX: str = 'python-gapic-' @@ -138,6 +138,7 @@ def tweak_path(p): lazy_import=bool(opts.pop('lazy-import', False)), old_naming=bool(opts.pop('old-naming', False)), add_iam_methods=bool(opts.pop('add-iam-methods', False)), + # transport should include desired transports delimited by '+', e.g. transport='grpc+rest' transport=opts.pop('transport', ['grpc'])[0].split('+') ) From 86f3a9c15fe03d15447c2adb864abee7c021ce7a Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Mon, 9 Nov 2020 22:59:28 +0000 Subject: [PATCH 06/14] test: added unit test for transport flag --- gapic/generator/generator.py | 5 +---- gapic/schema/wrappers.py | 7 +++--- tests/unit/generator/test_generator.py | 31 ++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/gapic/generator/generator.py b/gapic/generator/generator.py index 6460c3b182..9ee6d97cca 100644 --- a/gapic/generator/generator.py +++ b/gapic/generator/generator.py @@ -299,10 +299,7 @@ def _render_template( def _is_desired_transport(self, template_name: str, opts: Options) -> bool: """Returns true if template name contains a desired transport""" desired_transports = ['__init__', 'base'] + opts.transport - for transport in desired_transports: - if transport in template_name: - return True - return False + return any(transport in template_name for transport in desired_transports) def _get_file( self, diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 6b91a2cc40..5c793deb31 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -739,15 +739,14 @@ def field_headers(self) -> Sequence[str]: return next((tuple(pattern.findall(verb)) for verb in potential_verbs if verb), ()) @property - def http_opt(self) -> Dict[str, str]: + def http_opt(self) -> Optional[Dict[str, str]]: """Return the http option for this method.""" http = self.options.Extensions[annotations_pb2.http].ListFields() - answer: Dict[str, str] = None if len(http) < 1: - return answer + return None http_method = http[0] - answer = { + answer: Dict[str, str] = { 'method': http_method[0].name, 'url': http_method[1], } diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index a258c294cf..8df0167c52 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -115,6 +115,37 @@ def test_get_response_fails_invalid_file_paths(): ex_str = str(ex.value) assert "%proto" in ex_str and "%service" in ex_str +def test_get_response_ignores_unwanted_transports(): + g = make_generator() + with mock.patch.object(jinja2.FileSystemLoader, "list_templates") as lt: + lt.return_value = [ + "foo/%service/transports/river.py.j2", + "foo/%service/transports/car.py.j2", + "foo/%service/transports/grpc.py.j2", + "foo/%service/transports/__init__.py.j2", + "foo/%service/transports/base.py.j2", + "mollusks/squid/sample.py.j2", + ] + with mock.patch.object(jinja2.Environment, "get_template") as gt: + gt.return_value = jinja2.Template("Service: {{ service.name }}") + cgr = g.get_response(api_schema=make_api( + make_proto( + descriptor_pb2.FileDescriptorProto( + service=[ + descriptor_pb2.ServiceDescriptorProto( + name="SomeService"), + ] + ), + ) + ), + opts=Options.build("transport=river+car")) + assert len(cgr.file) == 4 + assert {i.name for i in cgr.file} == { + "foo/some_service/transports/river.py", + "foo/some_service/transports/car.py", + "foo/some_service/transports/__init__.py", + "foo/some_service/transports/base.py", + } def test_get_response_enumerates_services(): g = make_generator() From 234cc09a12b4e5fe46ebfbebe5093472711d6061 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Wed, 11 Nov 2020 13:25:20 +0000 Subject: [PATCH 07/14] test: add unit test for http option method --- .../services/%service/transports/grpc.py.j2 | 4 +++- tests/unit/generator/test_generator.py | 24 ++++++++++--------- tests/unit/schema/wrappers/test_method.py | 14 +++++++++++ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 index c7bd0cfc1e..3d1f5ca9b2 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 @@ -151,7 +151,9 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport): ) self._stubs = {} # type: Dict[str, Callable] - {%- if service.has_lro %} self._operations_client = None {%- endif %} + {%- if service.has_lro %} + self._operations_client = None + {%- endif %} # Run the base constructor. super().__init__( diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index 8df0167c52..e783a89fec 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -128,17 +128,19 @@ def test_get_response_ignores_unwanted_transports(): ] with mock.patch.object(jinja2.Environment, "get_template") as gt: gt.return_value = jinja2.Template("Service: {{ service.name }}") - cgr = g.get_response(api_schema=make_api( - make_proto( - descriptor_pb2.FileDescriptorProto( - service=[ - descriptor_pb2.ServiceDescriptorProto( - name="SomeService"), - ] - ), - ) - ), - opts=Options.build("transport=river+car")) + cgr = g.get_response( + api_schema=make_api( + make_proto( + descriptor_pb2.FileDescriptorProto( + service=[ + descriptor_pb2.ServiceDescriptorProto( + name="SomeService"), + ] + ), + ) + ), + opts=Options.build("transport=river+car") + ) assert len(cgr.file) == 4 assert {i.name for i in cgr.file} == { "foo/some_service/transports/river.py", diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py index 8b551df560..366b014220 100644 --- a/tests/unit/schema/wrappers/test_method.py +++ b/tests/unit/schema/wrappers/test_method.py @@ -248,6 +248,20 @@ def test_method_field_headers_present(): method = make_method('DoSomething', http_rule=rule) assert method.field_headers == ('parent',) +# TODO(yonmg) to test: grpc transcoding, +# correct handling of path/query params +# correct handling of body & additional binding +def test_method_http_opt(): + http_rule = http_pb2.HttpRule(post='/v1/{parent=projects/*}/topics', body='*') + method = make_method('DoSomething', http_rule=http_rule) + assert method.http_opt == {'method': 'post', + 'url': '/v1/{parent=projects/*}/topics', + 'body': '*'} + +def test_method_http_opt_no_http_rule(): + method = make_method('DoSomething') + assert method.http_opt == None + def test_method_idempotent_yes(): http_rule = http_pb2.HttpRule(get='/v1/{parent=projects/*}/topics') From 35ac2768bf667415ba24f0e11b3eddd4525359c6 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Wed, 11 Nov 2020 14:01:58 +0000 Subject: [PATCH 08/14] test: add unit test for http option method branch --- .../%service/transports/grpc_asyncio.py.j2 | 4 +++- tests/unit/schema/wrappers/test_method.py | 21 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 index d2b162d4e3..5ea7031162 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 @@ -205,7 +205,9 @@ class {{ service.grpc_asyncio_transport_name }}({{ service.name }}Transport): ) self._stubs = {} - {%- if service.has_lro %} self._operations_client = None {%- endif %} + {%- if service.has_lro %} + self._operations_client = None + {%- endif %} @property def grpc_channel(self) -> aio.Channel: diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py index 366b014220..62a546476c 100644 --- a/tests/unit/schema/wrappers/test_method.py +++ b/tests/unit/schema/wrappers/test_method.py @@ -252,11 +252,24 @@ def test_method_field_headers_present(): # correct handling of path/query params # correct handling of body & additional binding def test_method_http_opt(): - http_rule = http_pb2.HttpRule(post='/v1/{parent=projects/*}/topics', body='*') + http_rule = http_pb2.HttpRule( + post='/v1/{parent=projects/*}/topics', + body='*' + ) + method = make_method('DoSomething', http_rule=http_rule) + assert method.http_opt == { + 'method': 'post', + 'url': '/v1/{parent=projects/*}/topics', + 'body': '*' + } + +def test_method_http_opt_no_body(): + http_rule = http_pb2.HttpRule(post='/v1/{parent=projects/*}/topics') method = make_method('DoSomething', http_rule=http_rule) - assert method.http_opt == {'method': 'post', - 'url': '/v1/{parent=projects/*}/topics', - 'body': '*'} + assert method.http_opt == { + 'method': 'post', + 'url': '/v1/{parent=projects/*}/topics' + } def test_method_http_opt_no_http_rule(): method = make_method('DoSomething') From c8155a5fdea724a8df57b0b2feeb26d6964ceb58 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Wed, 11 Nov 2020 14:19:54 +0000 Subject: [PATCH 09/14] fix: fix import paths --- .../%name_%version/%sub/services/%service/client.py.j2 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index 5c58fdf5f7..cdbacde3c7 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -31,11 +31,11 @@ from google.iam.v1 import policy_pb2 as policy # type: ignore {% endfilter %} from .transports.base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO {%- if 'grpc' in opts.transport %} -from .grpc import {{ service.name }}GrpcTransport -from .grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport +from .transports.grpc import {{ service.name }}GrpcTransport +from .transports.grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport {%- endif %} {%- if 'rest' in opts.transport %} -from .rest import {{ service.name }}RestTransport +from .transports.rest import {{ service.name }}RestTransport {%- endif %} From 55a815cc83428d24136ecd1da231e92dead95e7c Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Wed, 11 Nov 2020 14:32:28 +0000 Subject: [PATCH 10/14] fix: style check issues --- gapic/schema/wrappers.py | 2 +- gapic/utils/options.py | 2 +- tests/unit/generator/test_generator.py | 4 +++- tests/unit/schema/wrappers/test_method.py | 9 ++++++--- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 5c793deb31..ea57fbe54f 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -737,7 +737,7 @@ def field_headers(self) -> Sequence[str]: ] return next((tuple(pattern.findall(verb)) for verb in potential_verbs if verb), ()) - + @property def http_opt(self) -> Optional[Dict[str, str]]: """Return the http option for this method.""" diff --git a/gapic/utils/options.py b/gapic/utils/options.py index df18c35619..25a0a8f09f 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -89,7 +89,7 @@ def build(cls, opt_string: str) -> 'Options': # Throw away other options not meant for us. if not opt.startswith(cls.PYTHON_GAPIC_PREFIX): continue - + # Set the option, using a key with the "python-gapic-" prefix # stripped. # diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index e783a89fec..18a64e15fa 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -115,6 +115,7 @@ def test_get_response_fails_invalid_file_paths(): ex_str = str(ex.value) assert "%proto" in ex_str and "%service" in ex_str + def test_get_response_ignores_unwanted_transports(): g = make_generator() with mock.patch.object(jinja2.FileSystemLoader, "list_templates") as lt: @@ -128,7 +129,7 @@ def test_get_response_ignores_unwanted_transports(): ] with mock.patch.object(jinja2.Environment, "get_template") as gt: gt.return_value = jinja2.Template("Service: {{ service.name }}") - cgr = g.get_response( + cgr = g.get_response( api_schema=make_api( make_proto( descriptor_pb2.FileDescriptorProto( @@ -149,6 +150,7 @@ def test_get_response_ignores_unwanted_transports(): "foo/some_service/transports/base.py", } + def test_get_response_enumerates_services(): g = make_generator() with mock.patch.object(jinja2.FileSystemLoader, "list_templates") as lt: diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py index 62a546476c..04946b3c52 100644 --- a/tests/unit/schema/wrappers/test_method.py +++ b/tests/unit/schema/wrappers/test_method.py @@ -248,9 +248,7 @@ def test_method_field_headers_present(): method = make_method('DoSomething', http_rule=rule) assert method.field_headers == ('parent',) -# TODO(yonmg) to test: grpc transcoding, -# correct handling of path/query params -# correct handling of body & additional binding + def test_method_http_opt(): http_rule = http_pb2.HttpRule( post='/v1/{parent=projects/*}/topics', @@ -262,6 +260,10 @@ def test_method_http_opt(): 'url': '/v1/{parent=projects/*}/topics', 'body': '*' } +# TODO(yonmg) to test: grpc transcoding, +# correct handling of path/query params +# correct handling of body & additional binding + def test_method_http_opt_no_body(): http_rule = http_pb2.HttpRule(post='/v1/{parent=projects/*}/topics') @@ -271,6 +273,7 @@ def test_method_http_opt_no_body(): 'url': '/v1/{parent=projects/*}/topics' } + def test_method_http_opt_no_http_rule(): method = make_method('DoSomething') assert method.http_opt == None From ac0bdef557be6b0d8dc5d21edd59266da1775a57 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Wed, 11 Nov 2020 14:34:44 +0000 Subject: [PATCH 11/14] fix: more style check issues --- gapic/utils/options.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index 25a0a8f09f..a327fa69b6 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -51,7 +51,8 @@ class Options: 'samples', # output dir 'lazy-import', # requires >= 3.7 'add-iam-methods', # microgenerator implementation for `reroute_to_grpc_interface` - 'transport', # transport type (i.e. grpc, rest, custom.[something], etc?) + # transport type (i.e. grpc, rest, custom.[something], etc?) + 'transport', )) @classmethod From e79e8c744d5728bcc299c8287b22209344780376 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Fri, 13 Nov 2020 20:04:42 +0000 Subject: [PATCH 12/14] fix: addressing pr reviews --- .../%namespace/%name/%version/__init__.py.j2 | 2 +- .../%namespace/%name/__init__.py.j2 | 2 +- gapic/cli/generate.py | 1 + gapic/generator/generator.py | 8 +++--- gapic/schema/wrappers.py | 27 ++++++++++++------- .../%sub/services/%service/async_client.py.j2 | 2 +- .../%sub/services/%service/client.py.j2 | 8 +++--- .../services/%service/transports/rest.py.j2 | 12 ++++----- gapic/utils/options.py | 4 +-- tests/unit/generator/test_generator.py | 2 ++ tests/unit/schema/wrappers/test_method.py | 2 +- 11 files changed, 41 insertions(+), 29 deletions(-) diff --git a/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 b/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 index ac517161fd..aa12751852 100644 --- a/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 +++ b/gapic/ads-templates/%namespace/%name/%version/__init__.py.j2 @@ -20,7 +20,7 @@ _lazy_type_to_package_map = { '{{ enum.name }}': '{{ enum.ident.package|join('.') }}.types.{{enum.ident.module }}', {%- endfor %} - {# TODO(yonmg): add rest transport service once I know what this is #} + {# TODO(yon-mg): add rest transport service once I know what this is #} # Client classes and transports {%- for service in api.services.values() %} '{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}', diff --git a/gapic/ads-templates/%namespace/%name/__init__.py.j2 b/gapic/ads-templates/%namespace/%name/__init__.py.j2 index ac517161fd..aa12751852 100644 --- a/gapic/ads-templates/%namespace/%name/__init__.py.j2 +++ b/gapic/ads-templates/%namespace/%name/__init__.py.j2 @@ -20,7 +20,7 @@ _lazy_type_to_package_map = { '{{ enum.name }}': '{{ enum.ident.package|join('.') }}.types.{{enum.ident.module }}', {%- endfor %} - {# TODO(yonmg): add rest transport service once I know what this is #} + {# TODO(yon-mg): add rest transport service once I know what this is #} # Client classes and transports {%- for service in api.services.values() %} '{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}', diff --git a/gapic/cli/generate.py b/gapic/cli/generate.py index 7950e462ff..921b2e9f11 100644 --- a/gapic/cli/generate.py +++ b/gapic/cli/generate.py @@ -42,6 +42,7 @@ def generate( # Pull apart arguments in the request. opts = Options.build(req.parameter) + # Determine the appropriate package. # This generator uses a slightly different mechanism for determining # which files to generate; it tracks at package level rather than file diff --git a/gapic/generator/generator.py b/gapic/generator/generator.py index 9ee6d97cca..45b204ce83 100644 --- a/gapic/generator/generator.py +++ b/gapic/generator/generator.py @@ -272,11 +272,11 @@ def _render_template( if "%service" in template_name: for service in api_schema.services.values(): if ( - skip_subpackages - and service.meta.address.subpackage != api_schema.subpackage_view + (skip_subpackages + and service.meta.address.subpackage != api_schema.subpackage_view) or - 'transport' in template_name - and not self._is_desired_transport(template_name, opts) + ('transport' in template_name + and not self._is_desired_transport(template_name, opts)) ): continue diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index ea57fbe54f..5a1763ec46 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -719,7 +719,8 @@ def _client_output(self, enable_asyncio: bool): # Return the usual output. return self.output - # TODO(yonmg): remove or rewrite. don't think it performs as intended + # TODO(yon-mg): remove or rewrite: don't think it performs as intended + # e.g. doesn't work with basic case of gRPC transcoding @property def field_headers(self) -> Sequence[str]: """Return the field headers defined for this method.""" @@ -740,25 +741,33 @@ def field_headers(self) -> Sequence[str]: @property def http_opt(self) -> Optional[Dict[str, str]]: - """Return the http option for this method.""" + """Return the http option for this method. + + e.g. {'verb': 'post' + 'url': '/some/path' + 'body': '*'} + + """ + http: List[Tuple[descriptor_pb2.FieldDescriptorProto, str]] http = self.options.Extensions[annotations_pb2.http].ListFields() + if len(http) < 1: return None http_method = http[0] answer: Dict[str, str] = { - 'method': http_method[0].name, + 'verb': http_method[0].name, 'url': http_method[1], } if len(http) > 1: - http_opt = http[1] - answer[http_opt[0].name] = http_opt[1] + body_spec = http[1] + answer[body_spec[0].name] = body_spec[1] - # TODO(yonmg): handle nested fields & fields past body i.e. 'additional bindings' - # TODO(yonmg): enums for http verbs? + # TODO(yon-mg): handle nested fields & fields past body i.e. 'additional bindings' + # TODO(yon-mg): enums for http verbs? return answer - # TODO(yonmg): refactor as there may be more than one method signature + # TODO(yon-mg): refactor as there may be more than one method signature @utils.cached_property def flattened_fields(self) -> Mapping[str, Field]: """Return the signature defined for this method.""" @@ -808,7 +817,7 @@ def grpc_stub_type(self) -> str: server='stream' if self.server_streaming else 'unary', ) - # TODO(yonmg): figure out why idempotent is reliant on http annotation + # TODO(yon-mg): figure out why idempotent is reliant on http annotation @utils.cached_property def idempotent(self) -> bool: """Return True if we know this method is idempotent, False otherwise. diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 index d851dec06f..c6320144ef 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 @@ -30,7 +30,7 @@ from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }} from .client import {{ service.client_name }} -{# TODO(yonmg): handle rest transport async client interaction #} +{# TODO(yon-mg): handle rest transport async client interaction #} class {{ service.async_client_name }}: """{{ service.meta.doc|rst(width=72, indent=4) }}""" diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index cdbacde3c7..6ec3d5d879 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -31,8 +31,8 @@ from google.iam.v1 import policy_pb2 as policy # type: ignore {% endfilter %} from .transports.base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO {%- if 'grpc' in opts.transport %} -from .transports.grpc import {{ service.name }}GrpcTransport -from .transports.grpc_asyncio import {{ service.name }}GrpcAsyncIOTransport +from .transports.grpc import {{ service.grpc_transport_name }} +from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }} {%- endif %} {%- if 'rest' in opts.transport %} from .transports.rest import {{ service.name }}RestTransport @@ -48,8 +48,8 @@ class {{ service.client_name }}Meta(type): """ _transport_registry = OrderedDict() # type: Dict[str, Type[{{ service.name }}Transport]] {%- if 'grpc' in opts.transport %} - _transport_registry['grpc'] = {{ service.name }}GrpcTransport - _transport_registry['grpc_asyncio'] = {{ service.name }}GrpcAsyncIOTransport + _transport_registry['grpc'] = {{ service.grpc_transport_name }} + _transport_registry['grpc_asyncio'] = {{ service.grpc_asyncio_transport_name }} {%- endif %} {%- if 'rest' in opts.transport %} _transport_registry['rest'] = {{ service.name }}RestTransport diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index 913f6a774e..d26856dd2c 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -16,7 +16,7 @@ import grpc # type: ignore from google.auth.transport.requests import AuthorizedSession -{# TODO: re-add python_import/ python_modules from removed diff/current grpc template code #} +{# TODO(yon-mg): re-add python_import/ python_modules from removed diff/current grpc template code #} {% filter sort_lines -%} {% for method in service.methods.values() -%} {{ method.input.ident.python_import }} @@ -42,7 +42,7 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): It sends JSON representations of protocol buffers over HTTP/1.1 """ - {# TODO(yonmg): handle mtls stuff if that's relevant for rest transport #} + {# TODO(yon-mg): handle mtls stuff if that's relevant for rest transport #} def __init__(self, *, host: str{% if service.host %} = '{{ service.host }}'{% endif %}, credentials: credentials.Credentials = None, @@ -144,8 +144,8 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): ) {%- endif %} - {# TODO(yonmg): Write helper method for handling grpc transcoding url #} - # TODO(yonmg): need to handle grpc transcoding and parse url correctly + {# TODO(yon-mg): Write helper method for handling grpc transcoding url #} + # TODO(yon-mg): need to handle grpc transcoding and parse url correctly # current impl assumes simpler version of grpc transcoding # Send the request url = 'https://{host}{{ method.http_opt['url'] }}'.format( @@ -154,7 +154,7 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): {{ field }}=request.{{ field }}, {%- endfor %} ) - {% if not method.void %}response = {% endif %}self._session.{{ method.http_opt['method'] }}( + {% if not method.void %}response = {% endif %}self._session.{{ method.http_opt['verb'] }}( url, {%- if 'body' in method.http_opt.keys() %} json=data, @@ -172,4 +172,4 @@ class {{ service.name }}RestTransport({{ service.name }}Transport): __all__ = ( '{{ service.name }}RestTransport', ) -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/gapic/utils/options.py b/gapic/utils/options.py index a327fa69b6..d99e34c631 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -14,7 +14,7 @@ from collections import defaultdict from os import path -from typing import Any, DefaultDict, Dict, FrozenSet, List, Optional, Tuple, Set +from typing import Any, DefaultDict, Dict, FrozenSet, List, Optional, Tuple import dataclasses import json @@ -51,7 +51,7 @@ class Options: 'samples', # output dir 'lazy-import', # requires >= 3.7 'add-iam-methods', # microgenerator implementation for `reroute_to_grpc_interface` - # transport type (i.e. grpc, rest, custom.[something], etc?) + # transport type(s) delineated by '+' (i.e. grpc, rest, custom.[something], etc?) 'transport', )) diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index 18a64e15fa..e68bf5bb66 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -127,6 +127,7 @@ def test_get_response_ignores_unwanted_transports(): "foo/%service/transports/base.py.j2", "mollusks/squid/sample.py.j2", ] + with mock.patch.object(jinja2.Environment, "get_template") as gt: gt.return_value = jinja2.Template("Service: {{ service.name }}") cgr = g.get_response( @@ -142,6 +143,7 @@ def test_get_response_ignores_unwanted_transports(): ), opts=Options.build("transport=river+car") ) + assert len(cgr.file) == 4 assert {i.name for i in cgr.file} == { "foo/some_service/transports/river.py", diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py index 04946b3c52..d462a7b5f1 100644 --- a/tests/unit/schema/wrappers/test_method.py +++ b/tests/unit/schema/wrappers/test_method.py @@ -260,7 +260,7 @@ def test_method_http_opt(): 'url': '/v1/{parent=projects/*}/topics', 'body': '*' } -# TODO(yonmg) to test: grpc transcoding, +# TODO(yon-mg) to test: grpc transcoding, # correct handling of path/query params # correct handling of body & additional binding From 824ad1167dd0ccb150b97f5f5659013f4b848bda Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Fri, 13 Nov 2020 22:48:14 +0000 Subject: [PATCH 13/14] fix: typo in test_method --- tests/unit/schema/wrappers/test_method.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py index d462a7b5f1..f6db3c044b 100644 --- a/tests/unit/schema/wrappers/test_method.py +++ b/tests/unit/schema/wrappers/test_method.py @@ -256,7 +256,7 @@ def test_method_http_opt(): ) method = make_method('DoSomething', http_rule=http_rule) assert method.http_opt == { - 'method': 'post', + 'verb': 'post', 'url': '/v1/{parent=projects/*}/topics', 'body': '*' } @@ -269,7 +269,7 @@ def test_method_http_opt_no_body(): http_rule = http_pb2.HttpRule(post='/v1/{parent=projects/*}/topics') method = make_method('DoSomething', http_rule=http_rule) assert method.http_opt == { - 'method': 'post', + 'verb': 'post', 'url': '/v1/{parent=projects/*}/topics' } From 2d11e19cb40dddef404f7ec7335c5fea30f85b91 Mon Sep 17 00:00:00 2001 From: Yonatan Getahun Date: Fri, 13 Nov 2020 22:59:27 +0000 Subject: [PATCH 14/14] fix: style check fixes --- gapic/cli/generate.py | 2 +- gapic/schema/wrappers.py | 2 +- tests/unit/generator/test_generator.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gapic/cli/generate.py b/gapic/cli/generate.py index 921b2e9f11..b0e50ccf2e 100644 --- a/gapic/cli/generate.py +++ b/gapic/cli/generate.py @@ -42,7 +42,7 @@ def generate( # Pull apart arguments in the request. opts = Options.build(req.parameter) - + # Determine the appropriate package. # This generator uses a slightly different mechanism for determining # which files to generate; it tracks at package level rather than file diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 5a1763ec46..664f240ffa 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -746,7 +746,7 @@ def http_opt(self) -> Optional[Dict[str, str]]: e.g. {'verb': 'post' 'url': '/some/path' 'body': '*'} - + """ http: List[Tuple[descriptor_pb2.FieldDescriptorProto, str]] http = self.options.Extensions[annotations_pb2.http].ListFields() diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index e68bf5bb66..97793e4433 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -143,7 +143,7 @@ def test_get_response_ignores_unwanted_transports(): ), opts=Options.build("transport=river+car") ) - + assert len(cgr.file) == 4 assert {i.name for i in cgr.file} == { "foo/some_service/transports/river.py",