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: implement async rest transport constructor #2123

Merged
merged 8 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ jobs:
strategy:
matrix:
python: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
variant: ['', _alternative_templates, _mixins, _alternative_templates_mixins]
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121) Remove `_w_rest_async` variant when async rest is GA.
variant: ['', _alternative_templates, _mixins, _alternative_templates_mixins, _w_rest_async]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down
9 changes: 6 additions & 3 deletions gapic/generator/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,12 @@ def _render_template(
('transport' in template_name
and not self._is_desired_transport(template_name, opts))
or
# TODO(yon-mg) - remove when rest async implementation resolved
# temporarily stop async client gen while rest async is unkown
('async' in template_name and 'grpc' not in opts.transport)
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): Remove this condition when async rest is GA.
('async_client' in template_name and 'grpc' not in opts.transport and
not api_schema.all_library_settings[api_schema.naming.proto_package].python_settings.experimental_features.rest_async_io_enabled)
or
('rest_asyncio' in template_name and
not api_schema.all_library_settings[api_schema.naming.proto_package].python_settings.experimental_features.rest_async_io_enabled)
or
('rest_base' in template_name and 'rest' not in opts.transport)
):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): Remove the following variable (and the condition later in this file) for async rest transport once support for it is GA. #}
{% set rest_async_io_enabled = api.all_library_settings[api.naming.proto_package].python_settings.experimental_features.rest_async_io_enabled %}
{% extends '_base.py.j2' %}

{% block content %}
Expand Down Expand Up @@ -62,6 +64,10 @@ from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }}
{% endif %}
{% if 'rest' in opts.transport %}
from .transports.rest import {{ service.name }}RestTransport
{# TODO(#2121): Remove this condition when async rest is GA. #}
{% if rest_async_io_enabled %}
from .transports.rest_asyncio import Async{{ service.name }}RestTransport
{% endif %}{# if rest_async_io_enabled #}
Comment on lines +68 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% if rest_async_io_enabled %}
from .transports.rest_asyncio import Async{{ service.name }}RestTransport
{% endif %}{# if rest_async_io_enabled #}
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): Remove this condition (and the variable set at the top of this file) for async rest transport once support for it is GA #}
{% if rest_async_io_enabled %}
from .transports.rest_asyncio import Async{{ service.name }}RestTransport
{% endif %}{# if rest_async_io_enabled #}

Copy link
Contributor

Choose a reason for hiding this comment

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

Also use the same format for these GA-TODOs in the other files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

{% endif %}


Expand All @@ -79,6 +85,10 @@ class {{ service.client_name }}Meta(type):
{% endif %}
{% if "rest" in opts.transport %}
_transport_registry["rest"] = {{ service.name }}RestTransport
{# TODO(#2121): Remove this condition when async rest is GA. #}
ohmayr marked this conversation as resolved.
Show resolved Hide resolved
{% if rest_async_io_enabled %}
Copy link
Contributor

Choose a reason for hiding this comment

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

{# TODO(#2121): Remove this condition when async rest is GA #}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL. GitHub shortened and hyperlinked the full URL I intended here.

_transport_registry["rest_asyncio"] = Async{{ service.name }}RestTransport
{% endif %}{# if rest_async_io_enabled #}
{% endif %}

def get_transport_class(cls,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): Remove the following variable (and the condition later in this file) for async rest transport once support for it is GA. #}
{% set rest_async_io_enabled = api.all_library_settings[api.naming.proto_package].python_settings.experimental_features.rest_async_io_enabled %}
{% extends '_base.py.j2' %}

{% block content %}

from google.api_core import gapic_v1

from typing import Any, Optional

from .rest_base import _Base{{ service.name }}RestTransport

from .base import DEFAULT_CLIENT_INFO as BASE_DEFAULT_CLIENT_INFO

{# TODO (https://github.com/googleapis/gapic-generator-python/issues/2128): Update `rest_version` to include the transport dependency version. #}
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=BASE_DEFAULT_CLIENT_INFO.gapic_version,
grpc_version=None,
rest_version=None,
)

class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport):
"""Asynchronous 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, *,
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
host: str{% if service.host %} = '{{ service.host }}'{% endif %},
{# TODO (https://github.com/googleapis/gapic-generator-python/issues/2129): Update the default type for credentials. #}
credentials: Optional[Any] = None,
client_info: gapic_v1.client_info.ClientInfo = DEFAULT_CLIENT_INFO,
url_scheme: str = 'https',
) -> None:
"""Instantiate the transport.

{% if not opts.rest_numeric_enums %}
NOTE: This async REST transport functionality is currently in a beta
state (preview). We welcome your feedback via a GitHub issue in
this library's repository. Thank you!
{% endif %}

Args:
host ({% if service.host %}Optional[str]{% else %}str{% endif %}):
{{ ' ' }}The hostname to connect to {% if service.host %}(default: '{{ service.host }}'){% endif %}.
{# TODO (https://github.com/googleapis/gapic-generator-python/issues/2129): Update the default type for credentials. #}
credentials (Optional[Any]): 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.
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 are developing
your own client library.
url_scheme (str): the protocol scheme for the API endpoint. Normally
"https", but for testing or local servers,
"http" can be specified.
"""
# Run the base constructor
super().__init__(
host=host,
credentials=credentials,
client_info=client_info,
always_use_jwt_access=False,
url_scheme=url_scheme,
api_audience=None
)

@property
def kind(self) -> str:
return "rest_asyncio"

{% endblock %}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): Remove the following variable (and the condition later in this file) for async rest transport once support for it is GA. #}
{% set rest_async_io_enabled = api.all_library_settings[api.naming.proto_package].python_settings.experimental_features.rest_async_io_enabled %}
{% extends "_base.py.j2" %}

{% block content %}
Expand Down Expand Up @@ -1041,9 +1043,17 @@ def test_transport_adc(transport_class):
transport_class()
adc.assert_called_once()

{{ test_macros.transport_kind_test(service, opts) }}

{{ test_macros.transport_kind_test(service, opts, is_async=True) }}
{% set configs = [] %}
parthea marked this conversation as resolved.
Show resolved Hide resolved
{% for transport_name in opts.transport %}
{% do configs.append({'service':service, 'transport_name':transport_name, 'is_async':false}) %}
{# TODO(#2121): Remove this condition when async rest is GA. #}
{% if 'grpc' in transport_name or rest_async_io_enabled %}
{% do configs.append({'service':service, 'transport_name':transport_name + "_asyncio", 'is_async':true}) %}
parthea marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% endfor %}
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
{% for conf in configs %}
{{ test_macros.transport_kind_test(**conf) }}
{% endfor %}

{% if 'grpc' in opts.transport %}
def test_transport_grpc_default():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1877,33 +1877,19 @@ def test_{{ method_name }}_empty_call():
assert args[0] == {{ method.input.ident }}()
{% endmacro %}

{% macro get_credentials(is_async=False) %}
{{-'async_anonymous_credentials()' if is_async else 'ga_credentials.AnonymousCredentials()'-}}
{% endmacro %}

{% macro transport_kind_test(service, opts, is_async=False) %}
@pytest.mark.parametrize("transport_name", [
{% if is_async %}
{% if "grpc" in opts.transport %}
"grpc_asyncio",
{% endif %}
{% else %}{# if not is_async #}
{% if "grpc" in opts.transport%}
"grpc",
{% endif %}
{% if "rest" in opts.transport %}
"rest",
{% endif %}
{% endif %}{# is_async #}
])
{% if is_async %}
@pytest.mark.asyncio
async def test_transport_kind_async(transport_name):
transport = {{ service.async_client_name }}.get_transport_class(transport_name)(
credentials=async_anonymous_credentials(),
)
{% else %}
def test_transport_kind(transport_name):
transport = {{ service.client_name }}.get_transport_class(transport_name)(
credentials=ga_credentials.AnonymousCredentials(),
{% macro get_client(service, is_async) %}
{{-service.async_client_name if is_async else service.client_name-}}
{% endmacro %}

{% macro transport_kind_test(service, transport_name, is_async) %}
def test_transport_kind_{{transport_name}}():
transport = {{ get_client(service, is_async) }}.get_transport_class("{{transport_name}}")(
credentials={{get_credentials(is_async)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% macro get_client(service, is_async) %}
{{-service.async_client_name if is_async else service.client_name-}}
{% endmacro %}
{% macro transport_kind_test(service, transport_name, is_async) %}
def test_transport_kind_{{transport_name}}():
transport = {{ get_client(service, is_async) }}.get_transport_class("{{transport_name}}")(
credentials={{get_credentials(is_async)}}
{% macro get_client(service, is_async) %}
{{- service.async_client_name if is_async else service.client_name -}}
{% endmacro %}
{% macro transport_kind_test(service, transport_name, is_async) %}
def test_transport_kind_{{ transport_name }}():
transport = {{ get_client(service, is_async) }}.get_transport_class("{{ transport_name }}")(
credentials={{ get_credentials(is_async) }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

)
{% endif %}
assert transport.kind == transport_name
{% endmacro %}
assert transport.kind == "{{ transport_name }}"

{% endmacro %}
51 changes: 51 additions & 0 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,31 @@ def fragment(session, use_ads_templates=False):
def fragment_alternative_templates(session):
fragment(session, use_ads_templates=True)

# `_add_python_settings` consumes a path to a temporary directory (str) i.e. tmp_dir and
# python settings i.e. python_settings (Dict) and modifies the service yaml within
# tmp_dir to include python settings. The primary purpose of this function is to modify
# the service yaml and include `rest_async_io_enabled=True` to test the async rest
# optional feature.
ohmayr marked this conversation as resolved.
Show resolved Hide resolved
def _add_python_settings(tmp_dir, python_settings):
return f"""
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
import yaml
from pathlib import Path
temp_file_path = Path(f"{tmp_dir}/showcase_v1beta1.yaml")
with temp_file_path.open('r') as file:
data = yaml.safe_load(file)
data['publishing']['library_settings'] = {python_settings}

with temp_file_path.open('w') as file:
yaml.safe_dump(data, file, default_flow_style=False, sort_keys=False)
"""

# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): `rest_async_io_enabled` must be removed once async rest is GA.
@contextmanager
def showcase_library(
session, templates="DEFAULT", other_opts: typing.Iterable[str] = (),
include_service_yaml=True,
retry_config=True,
rest_async_io_enabled=False
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should also have a TODO to remove the param once async REST is GA?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

):
"""Install the generated library into the session for showcase tests."""

Expand Down Expand Up @@ -220,6 +239,25 @@ def showcase_library(
external=True,
silent=True,
)
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): The section below updates the showcase service yaml
# to test experimental async rest transport. It must be removed once support for async rest is GA.
if rest_async_io_enabled:
# Install pyYAML for yaml.
session.install("pyYAML")

python_settings = [
{
'version': 'google.showcase.v1beta1',
'python_settings': {
'experimental_features': {
'rest_async_io_enabled': True
}
}
}
]
update_service_yaml = _add_python_settings(tmp_dir, python_settings)
session.run("python", "-c" f"{update_service_yaml}")
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
# END TODO section to remove.
if retry_config:
session.run(
"curl",
Expand Down Expand Up @@ -392,6 +430,19 @@ def showcase_unit(
run_showcase_unit_tests(session)


# TODO: `showcase_unit_w_rest_async` nox session runs showcase unit tests with the
# experimental async rest transport and must be removed once support for async rest is GA.
# See related issue: https://github.com/googleapis/gapic-generator-python/issues/2121.
@nox.session(python=ALL_PYTHON)
def showcase_unit_w_rest_async(
session, templates="DEFAULT", other_opts: typing.Iterable[str] = (),
):
"""Run the generated unit tests with async rest transport against the Showcase library."""
with showcase_library(session, templates=templates, other_opts=other_opts, rest_async_io_enabled=True) as lib:
session.chdir(lib)
run_showcase_unit_tests(session)


@nox.session(python=ALL_PYTHON)
def showcase_unit_alternative_templates(session):
with showcase_library(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16405,26 +16405,25 @@ def test_transport_adc(transport_class):
transport_class()
adc.assert_called_once()

@pytest.mark.parametrize("transport_name", [
"grpc",
"rest",
])
def test_transport_kind(transport_name):
transport = AssetServiceClient.get_transport_class(transport_name)(
credentials=ga_credentials.AnonymousCredentials(),
def test_transport_kind_grpc():
transport = AssetServiceClient.get_transport_class("grpc")(
credentials=ga_credentials.AnonymousCredentials()
)
assert transport.kind == transport_name
assert transport.kind == "grpc"


@pytest.mark.parametrize("transport_name", [
"grpc_asyncio",
])
@pytest.mark.asyncio
async def test_transport_kind_async(transport_name):
transport = AssetServiceAsyncClient.get_transport_class(transport_name)(
credentials=async_anonymous_credentials(),
def test_transport_kind_grpc_asyncio():
transport = AssetServiceAsyncClient.get_transport_class("grpc_asyncio")(
credentials=async_anonymous_credentials()
)
assert transport.kind == "grpc_asyncio"


def test_transport_kind_rest():
transport = AssetServiceClient.get_transport_class("rest")(
credentials=ga_credentials.AnonymousCredentials()
)
assert transport.kind == transport_name
assert transport.kind == "rest"


def test_transport_grpc_default():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3490,26 +3490,25 @@ def test_transport_adc(transport_class):
transport_class()
adc.assert_called_once()

@pytest.mark.parametrize("transport_name", [
"grpc",
"rest",
])
def test_transport_kind(transport_name):
transport = IAMCredentialsClient.get_transport_class(transport_name)(
credentials=ga_credentials.AnonymousCredentials(),
def test_transport_kind_grpc():
transport = IAMCredentialsClient.get_transport_class("grpc")(
credentials=ga_credentials.AnonymousCredentials()
)
assert transport.kind == transport_name
assert transport.kind == "grpc"


@pytest.mark.parametrize("transport_name", [
"grpc_asyncio",
])
@pytest.mark.asyncio
async def test_transport_kind_async(transport_name):
transport = IAMCredentialsAsyncClient.get_transport_class(transport_name)(
credentials=async_anonymous_credentials(),
def test_transport_kind_grpc_asyncio():
transport = IAMCredentialsAsyncClient.get_transport_class("grpc_asyncio")(
credentials=async_anonymous_credentials()
)
assert transport.kind == "grpc_asyncio"


def test_transport_kind_rest():
transport = IAMCredentialsClient.get_transport_class("rest")(
credentials=ga_credentials.AnonymousCredentials()
)
assert transport.kind == transport_name
assert transport.kind == "rest"


def test_transport_grpc_default():
Expand Down
Loading