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 3 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/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 @@ -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):
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
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

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