fix: Add retries when polling during monitoring runs - #786
sasha-gitg merged 4 commits into
Conversation
Sorry, something went wrong.
| ) | ||
|
|
||
| return getattr(self.api_client, self._getter_method)(name=resource_name) | ||
| return getattr(self.api_client, self._getter_method)( |
There was a problem hiding this comment.
How about delete and list methods?
Sorry, something went wrong.
There was a problem hiding this comment.
The purpose of the PR is to address intermittent failure during polling and the goal is to scope to that issue.
We can follow up on list and delete. delete will take additional care as the request to delete may get through to the service but we may not receive a response and retrying delete will throw an exception.
Sorry, something went wrong.
There was a problem hiding this comment.
I'm wondering if this would actually change anything as there is already a default retry associated with the client methods.
For example, EndpointServiceAsyncClient has:
async def get_endpoint(
self,
request: endpoint_service.GetEndpointRequest = None,
*,
name: str = None,
retry: retries.Retry = gapic_v1.method.DEFAULT,
timeout: float = None,
metadata: Sequence[Tuple[str, str]] = (),
) -> endpoint.Endpoint:
Is the default retry not sufficient?
Sorry, something went wrong.
| logging.basicConfig(level=logging.INFO, stream=sys.stdout) | ||
|
|
||
| # This is the default retry callback to be used with get methods. | ||
| _DEFAULT_RETRY = retry.Retry() |
There was a problem hiding this comment.
IIUC gapic_v1.method.DEFAULT is used when no retry is provided (https://github.com/googleapis/python-aiplatform/search?q=retry%3D). I wonder if there is any difference between _DEFAULT_RETRY and gapic_v1.method.DEFAULT?
If there is no difference, I think we need a more aggressive _DEFAULT_RETRY to fix the current problem.
Sorry, something went wrong.
There was a problem hiding this comment.
gapic_v1.method.DEFAULT is a sentinel value that is used to set the default retry passed into the constructor of the Gapic callable.
See usage here: https://github.com/googleapis/python-api-core/blob/main/google/api_core/gapic_v1/method.py#L125
The default retry is None: https://github.com/googleapis/python-api-core/blob/main/google/api_core/gapic_v1/method.py#L147
cc: @ivanmkc
Sorry, something went wrong.
|
LGTM, thanks Sasha! |
Sorry, something went wrong.
ivanmkc
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
b/200047992