-
Notifications
You must be signed in to change notification settings - Fork 77
feat: wrap async rest methods with retry and error logic #2139
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
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 |
|---|---|---|
|
|
@@ -14,15 +14,22 @@ | |
| # limitations under the License. | ||
| # | ||
| 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 | ||
| raise ImportError("async rest transport requires google.auth >= 2.x.x") from e | ||
|
|
||
| from google.auth.aio import credentials as ga_credentials_async # type: ignore | ||
|
|
||
| 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 | ||
|
|
||
| from typing import Any, Callable, Tuple, Optional, Sequence, Union | ||
|
|
||
|
|
||
| from google.cloud.redis_v1.types import cloud_redis | ||
| from google.longrunning import operations_pb2 # type: ignore | ||
|
|
||
| from typing import Any, Optional | ||
|
|
||
| from .rest_base import _BaseCloudRedisRestTransport | ||
|
|
||
|
|
@@ -104,6 +111,139 @@ def __init__(self, *, | |
| api_audience=None | ||
| ) | ||
| self._session = AsyncAuthorizedSession(self._credentials) | ||
| self._wrap_with_kind = True | ||
| self._prep_wrapped_messages(client_info) | ||
|
|
||
| def _prep_wrapped_messages(self, client_info): | ||
| """ Precompute the wrapped methods, overriding the base class method to use async wrappers.""" | ||
| self._wrapped_methods = { | ||
| self.list_instances: self._wrap_method( | ||
| self.list_instances, | ||
|
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. nit: It would be good if we could test a method with retry settings, since those are used in
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. Good point! I think it won't be that straightforward to test retry logic before the actual call methods are implemented (for async rest) . I also think that we should add similar tests for async gRPC. Filed: #2155
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. Retry logic will be tested as part of e2e showcase testing (once we turn them on for async REST) which is something that I just remembered. |
||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.get_instance: self._wrap_method( | ||
| self.get_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.get_instance_auth_string: self._wrap_method( | ||
| self.get_instance_auth_string, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.create_instance: self._wrap_method( | ||
| self.create_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.update_instance: self._wrap_method( | ||
| self.update_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.upgrade_instance: self._wrap_method( | ||
| self.upgrade_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.import_instance: self._wrap_method( | ||
| self.import_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.export_instance: self._wrap_method( | ||
| self.export_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.failover_instance: self._wrap_method( | ||
| self.failover_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.delete_instance: self._wrap_method( | ||
| self.delete_instance, | ||
| default_timeout=600.0, | ||
| client_info=client_info, | ||
| ), | ||
| self.reschedule_maintenance: self._wrap_method( | ||
| self.reschedule_maintenance, | ||
| default_timeout=None, | ||
| client_info=client_info, | ||
| ), | ||
| } | ||
|
|
||
| def _wrap_method(self, func, *args, **kwargs): | ||
| if self._wrap_with_kind: # pragma: NO COVER | ||
| kwargs["kind"] = self.kind | ||
| return gapic_v1.method_async.wrap_method(func, *args, **kwargs) | ||
|
|
||
| @property | ||
| def create_instance(self) -> Callable[ | ||
| [cloud_redis.CreateInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # 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. Please add a comment to clarify the reason that we have
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. +1
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. addressed.
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. Not seeing it...
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. The comment's added in the jinja template and not the generated file:
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. Also a related thread from the PR above it: #2140 (comment). |
||
|
|
||
| @property | ||
| def delete_instance(self) -> Callable[ | ||
| [cloud_redis.DeleteInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def export_instance(self) -> Callable[ | ||
| [cloud_redis.ExportInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def failover_instance(self) -> Callable[ | ||
| [cloud_redis.FailoverInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def get_instance(self) -> Callable[ | ||
| [cloud_redis.GetInstanceRequest], | ||
| cloud_redis.Instance]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def get_instance_auth_string(self) -> Callable[ | ||
| [cloud_redis.GetInstanceAuthStringRequest], | ||
| cloud_redis.InstanceAuthString]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def import_instance(self) -> Callable[ | ||
| [cloud_redis.ImportInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def list_instances(self) -> Callable[ | ||
| [cloud_redis.ListInstancesRequest], | ||
| cloud_redis.ListInstancesResponse]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def reschedule_maintenance(self) -> Callable[ | ||
| [cloud_redis.RescheduleMaintenanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def update_instance(self) -> Callable[ | ||
| [cloud_redis.UpdateInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def upgrade_instance(self) -> Callable[ | ||
| [cloud_redis.UpgradeInstanceRequest], | ||
| operations_pb2.Operation]: | ||
| return # type: ignore | ||
|
|
||
| @property | ||
| def kind(self) -> str: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.