feat: implement async rest transport constructor #2123

ohmayr · 2024-09-01T03:02:24Z

This PR introduces an async rest transport with minimal parameters and a kind property. It also sets up the testing infra for async rest i.e:

add a new nox session to test async rest transport.
updates the github workflow to include the new nox session.
adds a list of configurations to run macros against.
update the redis golden service yaml file to include the experimental feature flag
update the service yaml for showcase within the noxfile for testing the new feature against the added feature flag.

.github/workflows/tests.yaml

vchudnov-g · 2024-09-03T22:41:28Z

.github/workflows/tests.yaml

@@ -140,7 +140,7 @@ jobs:
    strategy:
      matrix:
        python: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
-        variant: ['', _alternative_templates, _mixins, _alternative_templates_mixins]
+        variant: ['', _alternative_templates, _mixins, _alternative_templates_mixins, "_w_rest_async"]


It seems like the previous entries were a comprehensive listing of various combinations (on/off along two axes: alternate_templates, mixins). Now it seems like you're adding another axis, async_rest, so we should have 8 entries to test all combinations. (In other words, with _w_rest_async, what are we using for template type and for mixins?

Ideally, we would have a 2x2x2 matrix here of (template-type, mixins, rest_async). That said, we don't want to get too distracted from the async work right now, so maybe simply a comment about what this variant includes/excludes is fine for now.

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

vchudnov-g · 2024-09-03T22:47:17Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

+{% if rest_async_io_enabled %}
+from .transports.rest_asyncio import Async{{ service.name }}RestTransport
+{% endif %}{# if rest_async_io_enabled #}


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 #}

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

vchudnov-g · 2024-09-03T22:48:52Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

@@ -79,6 +87,9 @@ class {{ service.client_name }}Meta(type):
    {% endif %}
    {% if "rest" in opts.transport %}
    _transport_registry["rest"] = {{ service.name }}RestTransport
+    {% if rest_async_io_enabled %}


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

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

vchudnov-g · 2024-09-03T23:31:09Z

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2

+{% macro transport_kind_test(service, transport_name) %}
+{%- set named_clients = {


Suggested change

{% macro transport_kind_test(service, transport_name) %}

{%- set named_clients = {

{% macro transport_kind_test(service, transport_name) %}

{# Look-up tables for parameter values corresponding to each possible transport-name #}

{%- set named_clients = {

I ended up removing this dict since there's only two possible clients that we can have i.e. sync or async. Added a separate macro for it.

Looks good!

noxfile.py

vchudnov-g · 2024-09-03T23:39:08Z

noxfile.py

 @contextmanager
 def showcase_library(
    session, templates="DEFAULT", other_opts: typing.Iterable[str] = (),
    include_service_yaml=True,
    retry_config=True,
+    rest_async_io_enabled=False


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

noxfile.py

tests/integration/redis_v1.yaml

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2

parthea · 2024-09-04T20:19:32Z

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2

+{% 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)}}


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) }}

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2

noxfile.py

vchudnov-g · 2024-09-05T18:32:03Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

@@ -79,6 +87,9 @@ class {{ service.client_name }}Meta(type):
    {% endif %}
    {% if "rest" in opts.transport %}
    _transport_registry["rest"] = {{ service.name }}RestTransport
+    {% if rest_async_io_enabled %}


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

vchudnov-g

Thanks for doing this! Looks great!

product-auto-label bot added size: m Pull request size is medium. size: l Pull request size is large. and removed size: m Pull request size is medium. labels Sep 1, 2024

ohmayr force-pushed the introduce-async-transport branch from 5f258b8 to 9b45d08 Compare September 1, 2024 20:01

ohmayr changed the title ~~initial setup: implement async rest transport constructor~~ setup: implement async rest transport constructor Sep 1, 2024

ohmayr force-pushed the introduce-async-transport branch from 90999f3 to 7d58f1f Compare September 1, 2024 20:44

ohmayr changed the base branch from main to async-rest-initial September 1, 2024 20:46

ohmayr force-pushed the introduce-async-transport branch 2 times, most recently from b90b18f to 7124af4 Compare September 1, 2024 20:52

ohmayr changed the title ~~setup: implement async rest transport constructor~~ feat: implement async rest transport constructor Sep 1, 2024

ohmayr force-pushed the introduce-async-transport branch 5 times, most recently from 097aa19 to 9f9471d Compare September 2, 2024 17:12

ohmayr marked this pull request as ready for review September 3, 2024 17:26

ohmayr requested a review from a team as a code owner September 3, 2024 17:26

blunderbuss-gcf bot assigned parthea Sep 3, 2024

ohmayr assigned vchudnov-g Sep 3, 2024

ohmayr force-pushed the introduce-async-transport branch from 9f9471d to f4f941c Compare September 3, 2024 19:48

ohmayr force-pushed the async-rest-initial branch from fbceb22 to dd34d55 Compare September 3, 2024 20:20

ohmayr force-pushed the introduce-async-transport branch 2 times, most recently from 2c28ab3 to 6cad5b9 Compare September 3, 2024 20:33

ohmayr force-pushed the async-rest-initial branch from dd34d55 to aa4bfa6 Compare September 3, 2024 20:35

chore: optionally generate async rest and client files

d9f8d2b

ohmayr force-pushed the async-rest-initial branch from aa4bfa6 to d9f8d2b Compare September 3, 2024 20:36

feat: implement async rest transport constructor

fcc89de

ohmayr force-pushed the introduce-async-transport branch from 6cad5b9 to fcc89de Compare September 3, 2024 20:42

vchudnov-g reviewed Sep 3, 2024

View reviewed changes

parthea mentioned this pull request Sep 4, 2024

chore: optionally generate async rest and client files #2122

Merged

parthea reviewed Sep 4, 2024

View reviewed changes

ohmayr added 3 commits September 4, 2024 19:31

address PR comments

0bbb9ab

address feedback

d48c6cf

address minor nits

41ded04

parthea approved these changes Sep 4, 2024

View reviewed changes

parthea reviewed Sep 4, 2024

View reviewed changes

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 Outdated Show resolved Hide resolved

address PR comments

1806b89

parthea approved these changes Sep 4, 2024

View reviewed changes

Base automatically changed from async-rest-initial to main September 4, 2024 21:16

Merge branch 'main' into introduce-async-transport

0a3e32f

vchudnov-g reviewed Sep 5, 2024

View reviewed changes

vchudnov-g approved these changes Sep 5, 2024

View reviewed changes

address PR comments

c26f72c

ohmayr merged commit 2809753 into main Sep 5, 2024
73 checks passed

ohmayr deleted the introduce-async-transport branch September 5, 2024 19:08

release-please bot mentioned this pull request Sep 5, 2024

chore(main): release 1.19.0 #2108

Merged

parthea mentioned this pull request Sep 20, 2024

[async rest] Add kind property to transport in ads templates #2183

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement async rest transport constructor #2123

feat: implement async rest transport constructor #2123

ohmayr commented Sep 1, 2024 •

edited

Loading

vchudnov-g Sep 3, 2024

vchudnov-g Sep 3, 2024

vchudnov-g Sep 3, 2024

ohmayr Sep 4, 2024

vchudnov-g Sep 3, 2024

ohmayr Sep 4, 2024

vchudnov-g Sep 5, 2024

vchudnov-g Sep 3, 2024

ohmayr Sep 4, 2024

vchudnov-g Sep 5, 2024

vchudnov-g Sep 3, 2024

ohmayr Sep 4, 2024

parthea Sep 4, 2024

ohmayr Sep 4, 2024

vchudnov-g Sep 5, 2024

vchudnov-g left a comment

		{% macro transport_kind_test(service, transport_name) %}
		{%- set named_clients = {

feat: implement async rest transport constructor #2123

feat: implement async rest transport constructor #2123

Conversation

ohmayr commented Sep 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vchudnov-g left a comment

Choose a reason for hiding this comment

ohmayr commented Sep 1, 2024 •

edited

Loading