feat: wrap async rest methods with retry and error logic #2139
Conversation
|
NOTE: The base branch of this PR should be updated to |
ohmayr
force-pushed
the
wrap-async-methods-with-error-handling-logic
branch
from
9b98c48
to
8475e46
Compare
ohmayr
force-pushed
the
wrap-async-methods-with-error-handling-logic
branch
from
8475e46
to
5fd24b6
Compare
ohmayr
force-pushed
the
wrap-async-methods-with-error-handling-logic
branch
from
5fd24b6
to
4c5a9c6
Compare
| [cloud_redis.CreateInstanceRequest], | ||
| operations_pb2.Operation]: | ||
|
|
||
| return # type: ignore |
There was a problem hiding this comment.
Please add a comment to clarify the reason that we have # type: ignore here and add TODO bug to follow up on it
There was a problem hiding this comment.
The comment's added in the jinja template and not the generated file:
https://github.com/googleapis/gapic-generator-python/pull/2139/files#diff-95a8ad0682619d6c91209ed703a5d69b195ea28be2711b516d956265695acc14R96
There was a problem hiding this comment.
Also a related thread from the PR above it: #2140 (comment).
ohmayr
force-pushed
the
wrap-async-methods-with-error-handling-logic
branch
from
79cc77d
to
23cb93e
Compare
...egration/goldens/redis/google/cloud/redis_v1/services/cloud_redis/transports/rest_asyncio.py
Show resolved
Hide resolved
| [cloud_redis.CreateInstanceRequest], | ||
| operations_pb2.Operation]: | ||
|
|
||
| return # type: ignore |
| def create_instance(self) -> Callable[ | ||
| [cloud_redis.CreateInstanceRequest], | ||
| operations_pb2.Operation]: | ||
|
|
There was a problem hiding this comment.
nit: remove this empty blank line for now
| """ 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, |
There was a problem hiding this comment.
nit: It would be good if we could test a method with retry settings, since those are used in prep_wrapped_messages_async_method
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| [cloud_redis.CreateInstanceRequest], | ||
| operations_pb2.Operation]: | ||
|
|
||
| return # type: ignore |
This PR adds error handling and retry logic to async rest methods. Note that the
callableswill be implemented in a follow up PR.This PR should be reviewed and merged after: #2138.