Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add rest transport generation for clients with optional flag #688

Merged
merged 16 commits into from
Nov 14, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 #}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use your github id for TODOs (here and in the other places)

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

# Client classes and transports
{%- for service in api.services.values() %}
'{{ service.client_name }}': '{{ service.meta.address.package|join('.') }}.services.{{ service.meta.address.module }}',
Expand Down
1 change: 1 addition & 0 deletions gapic/ads-templates/%namespace/%name/__init__.py.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}',
Expand Down
4 changes: 4 additions & 0 deletions gapic/generator/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
software-dov marked this conversation as resolved.
Show resolved Hide resolved
or
'rest' in template_name and 'rest' not in opts.transport
):
continue

Expand Down
4 changes: 4 additions & 0 deletions gapic/schema/wrappers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }}"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +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 }}
{%- 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):
Expand All @@ -42,8 +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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +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 %}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
software-dov marked this conversation as resolved.
Show resolved Hide resolved
address (Optional[str]): The host for the channel to use.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this changed (looks unrelated to your changes)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor

Choose a reason for hiding this comment

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

I approve of fixing typos on the fly!

credentials (Optional[~.Credentials]): The
authorization credentials to attach to requests. These
credentials identify this application to the service. If
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
software-dov marked this conversation as resolved.
Show resolved Hide resolved
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']
software-dov marked this conversation as resolved.
Show resolved Hide resolved
{%- endif %}


{%- for method in service.methods.values() %}
{# TODO(yonmg): consider using enums #}
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for using enums.

{%- set ns = namespace(http=None) %}
software-dov marked this conversation as resolved.
Show resolved Hide resolved
{%- for option in method.options.ListFields() %}
{%- if option[0].name == 'http' %}
{%- set ns.http = option[1] %}
{%- endif %}
{%- endfor %}
software-dov marked this conversation as resolved.
Show resolved Hide resolved

{%- 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 %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm leaning towards anything that involves state modification living in the generator datatypes. It's easier to test and easier to change if we need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand what you mean by generator datatypes. Could you clarify?

Copy link
Contributor

Choose a reason for hiding this comment

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

The gapic/schema/wrappers.py:{MessageType,Service,Method} classes. The auxiliary datastructures that the generate creates before iterating over the templates.


{%- 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 %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add new line

9 changes: 7 additions & 2 deletions gapic/utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Set used somewhere?


import dataclasses
import json
Expand All @@ -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?
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for using an enum

transport: List[str] = None

# Class constants
PYTHON_GAPIC_PREFIX: str = 'python-gapic-'
Expand All @@ -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
Expand Down Expand Up @@ -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.
#
Expand Down Expand Up @@ -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', [])),
Expand All @@ -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('+')
software-dov marked this conversation as resolved.
Show resolved Hide resolved
)

# Note: if we ever need to recursively check directories for sample
Expand Down
1 change: 1 addition & 0 deletions tests/unit/schema/wrappers/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down