fix: Add retries when polling during monitoring runs #786
Conversation
product-auto-label
bot
added
the
api: aiplatform
google-cla
bot
added
the
cla: yes
| @@ -532,7 +536,9 @@ def _get_gca_resource(self, resource_name: str) -> proto.Message: | |||
| location=self.location, | |||
| ) | |||
|
|
|||
| 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?
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.
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?
| @@ -48,6 +49,9 @@ | |||
|
|
|||
| 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.
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
|
LGTM, thanks Sasha! |
b/200047992