-
Notifications
You must be signed in to change notification settings - Fork 77
feat: add async rest transport call methods #2140
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| {% block content %} | ||
|
|
||
| try: | ||
| from google.auth.aio.transport.sessions import AsyncAuthorizedSession # type: ignore | ||
| from google.auth.aio.transport.sessions import AsyncAuthorizedSession # type: ignore | ||
| except ImportError as e: # pragma: NO COVER | ||
| {# TODO(https://github.com/googleapis/google-auth-library-python/pull/1577): Update the version of google-auth once the linked PR is merged. #} | ||
| raise ImportError("async rest transport requires google.auth >= 2.x.x") from e | ||
|
|
@@ -17,6 +17,7 @@ from google.api_core import exceptions as core_exceptions | |
| from google.api_core import gapic_v1 | ||
| from google.api_core import retry_async as retries | ||
|
|
||
| import dataclasses | ||
| from typing import Any, Callable, Tuple, Optional, Sequence, Union | ||
|
|
||
| {{ shared_macros.operations_mixin_imports(api, service, opts) }} | ||
|
|
@@ -25,13 +26,24 @@ from .rest_base import _Base{{ service.name }}RestTransport | |
|
|
||
| from .base import DEFAULT_CLIENT_INFO as BASE_DEFAULT_CLIENT_INFO | ||
|
|
||
| try: | ||
| OptionalRetry = Union[retries.AsyncRetry, gapic_v1.method._MethodDefault, None] | ||
| except AttributeError: # pragma: NO COVER | ||
| OptionalRetry = Union[retries.AsyncRetry, object, None] # type: ignore | ||
|
|
||
| {# 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, | ||
| ) | ||
|
|
||
| {# TODO: Add an `_interceptor` property once implemented #} | ||
| @dataclasses.dataclass | ||
| class Async{{service.name}}RestStub: | ||
| _session: AsyncAuthorizedSession | ||
| _host: str | ||
|
|
||
| class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): | ||
| """Asynchronous REST backend transport for {{ service.name }}. | ||
|
|
||
|
|
@@ -92,14 +104,29 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): | |
| {{ shared_macros.wrap_async_method_macro()|indent(4) }} | ||
|
|
||
| {% for method in service.methods.values()|sort(attribute="name") %} | ||
| class _{{method.name}}(_Base{{ service.name }}RestTransport._Base{{method.name}}, Async{{service.name}}RestStub): | ||
| def __hash__(self): | ||
| return hash("Async{{service.name}}RestTransport.{{method.name}}") | ||
|
|
||
| async def __call__(self, | ||
| request: {{method.input.ident}}, *, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does the asterisk mean here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, it's a Python thing. Because it was on the same line, I assumed it was part of the type hint and glossed over the comma. I suggest having the |
||
| retry: OptionalRetry=gapic_v1.method.DEFAULT, | ||
| timeout: Optional[float]=None, | ||
| metadata: Sequence[Tuple[str, str]]=(), | ||
| {# TODO(b/362949446): Update the return type as we implement this for different method types. #} | ||
| ) -> None: | ||
| raise NotImplementedError( | ||
| "Method {{ method.name }} is not available over REST transport" | ||
| ) | ||
|
|
||
| {# TODO(b/362949446) Return a callable once the class is implemented. #} | ||
| {% endfor %} | ||
| {% for method in service.methods.values()|sort(attribute="name") %} | ||
| {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2154): Remove `type: ignore`. #} | ||
| @property | ||
| def {{method.transport_safe_name|snake_case}}(self) -> Callable[ | ||
| [{{method.input.ident}}], | ||
| {{method.output.ident}}]: | ||
| return # type: ignore | ||
| return self._{{method.name}}(self._session, self._host) # type: ignore | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. My initial thought was that it's probably a mismatch since I wasn't returning a Callable. However, I later realised that we're doing the same in I haven't really looked into why we've been ignoring the type in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So each of these properties is a callable that invokes the appropriate inner class. When do the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrapping happens during transport initialization / construction via At the client level instead of calling
FYI these are mixin methods. We don't pre-wrap mixin methods which is why this logic isn't added. We probably should do that or at least pass down the kind property to avoid gRPC code path in a rest call at the very least.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is important. I've filed https://github.com/googleapis/gapic-generator-python/issues/2156 to ensure that we do this as a follow up since this PR doesn't touch the client layer.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
OK, this is what I was looking for. Once we have |
||
|
|
||
| {% endfor %} | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include the TODO bug to follow up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to ignore this one since this comment is addressed and removed in a follow up PR: #2151.