From e0c1b4514d97de6fc93b87197125c53947dff442 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 13 Feb 2025 15:58:41 -0800 Subject: [PATCH 01/10] feat: retry generates backoff value after completing on_error callbacks --- google/api_core/retry/retry_base.py | 15 +++++++++++---- google/api_core/retry/retry_streaming.py | 10 ++++------ google/api_core/retry/retry_streaming_async.py | 10 +++++----- google/api_core/retry/retry_unary.py | 10 ++++------ google/api_core/retry/retry_unary_async.py | 10 ++++------ 5 files changed, 28 insertions(+), 27 deletions(-) diff --git a/google/api_core/retry/retry_base.py b/google/api_core/retry/retry_base.py index 1606e0fe..4244e362 100644 --- a/google/api_core/retry/retry_base.py +++ b/google/api_core/retry/retry_base.py @@ -25,7 +25,7 @@ import time from enum import Enum -from typing import Any, Callable, Optional, TYPE_CHECKING +from typing import Any, Callable, Optional, Iterable, TYPE_CHECKING import requests.exceptions @@ -174,7 +174,7 @@ def build_retry_error( def _retry_error_helper( exc: Exception, deadline: float | None, - next_sleep: float, + sleep_generator: Iterable[float], error_list: list[Exception], predicate_fn: Callable[[Exception], bool], on_error_fn: Callable[[Exception], None] | None, @@ -183,7 +183,7 @@ def _retry_error_helper( tuple[Exception, Exception | None], ], original_timeout: float | None, -): +) -> float: """ Shared logic for handling an error for all retry implementations @@ -194,13 +194,15 @@ def _retry_error_helper( Args: - exc: the exception that was raised - deadline: the deadline for the retry, calculated as a diff from time.monotonic() - - next_sleep: the next sleep interval + - sleep_generator: iterable to draw the next backoff value from - error_list: the list of exceptions that have been raised so far - predicate_fn: takes `exc` and returns true if the operation should be retried - on_error_fn: callback to execute when a retryable error occurs - exc_factory_fn: callback used to build the exception to be raised on terminal failure - original_timeout_val: the original timeout value for the retry (in seconds), to be passed to the exception factory for building an error message + Returns: + - the next backoff value to use """ error_list.append(exc) if not predicate_fn(exc): @@ -212,6 +214,10 @@ def _retry_error_helper( raise final_exc from source_exc if on_error_fn is not None: on_error_fn(exc) + try: + next_sleep = next(sleep_generator) + except StopIteration: + raise ValueError("Sleep generator stopped yielding sleep values.") from exc if deadline is not None and time.monotonic() + next_sleep > deadline: final_exc, source_exc = exc_factory_fn( error_list, @@ -222,6 +228,7 @@ def _retry_error_helper( _LOGGER.debug( "Retrying due to {}, sleeping {:.1f}s ...".format(error_list[-1], next_sleep) ) + return next_sleep class _BaseRetry(object): diff --git a/google/api_core/retry/retry_streaming.py b/google/api_core/retry/retry_streaming.py index e113323b..5711cbba 100644 --- a/google/api_core/retry/retry_streaming.py +++ b/google/api_core/retry/retry_streaming.py @@ -108,7 +108,7 @@ def retry_target_stream( ) error_list: list[Exception] = [] - for sleep in sleep_generator: + while True: # Start a new retry loop try: # Note: in the future, we can add a ResumptionStrategy object @@ -121,10 +121,10 @@ def retry_target_stream( # This function explicitly must deal with broad exceptions. except Exception as exc: # defer to shared logic for handling errors - _retry_error_helper( + next_sleep = _retry_error_helper( exc, deadline, - sleep, + sleep_generator, error_list, predicate, on_error, @@ -132,9 +132,7 @@ def retry_target_stream( timeout, ) # if exception not raised, sleep before next attempt - time.sleep(sleep) - - raise ValueError("Sleep generator stopped yielding sleep values.") + time.sleep(next_sleep) class StreamingRetry(_BaseRetry): diff --git a/google/api_core/retry/retry_streaming_async.py b/google/api_core/retry/retry_streaming_async.py index 2924ba14..2dc98179 100644 --- a/google/api_core/retry/retry_streaming_async.py +++ b/google/api_core/retry/retry_streaming_async.py @@ -111,7 +111,7 @@ async def retry_target_stream( error_list: list[Exception] = [] target_is_generator: bool | None = None - for sleep in sleep_generator: + while True: # Start a new retry loop try: # Note: in the future, we can add a ResumptionStrategy object @@ -174,10 +174,10 @@ async def retry_target_stream( # This function explicitly must deal with broad exceptions. except Exception as exc: # defer to shared logic for handling errors - _retry_error_helper( + next_sleep = _retry_error_helper( exc, deadline, - sleep, + sleep_generator, error_list, predicate, on_error, @@ -185,11 +185,11 @@ async def retry_target_stream( timeout, ) # if exception not raised, sleep before next attempt - await asyncio.sleep(sleep) + await asyncio.sleep(next_sleep) + finally: if target_is_generator and target_iterator is not None: await cast(AsyncGenerator["_Y", None], target_iterator).aclose() - raise ValueError("Sleep generator stopped yielding sleep values.") class AsyncStreamingRetry(_BaseRetry): diff --git a/google/api_core/retry/retry_unary.py b/google/api_core/retry/retry_unary.py index d5dff663..86552004 100644 --- a/google/api_core/retry/retry_unary.py +++ b/google/api_core/retry/retry_unary.py @@ -139,7 +139,7 @@ def retry_target( deadline = time.monotonic() + timeout if timeout is not None else None error_list: list[Exception] = [] - for sleep in sleep_generator: + while True: try: result = target() if inspect.isawaitable(result): @@ -150,10 +150,10 @@ def retry_target( # This function explicitly must deal with broad exceptions. except Exception as exc: # defer to shared logic for handling errors - _retry_error_helper( + next_sleep = _retry_error_helper( exc, deadline, - sleep, + sleep_generator, error_list, predicate, on_error, @@ -161,9 +161,7 @@ def retry_target( timeout, ) # if exception not raised, sleep before next attempt - time.sleep(sleep) - - raise ValueError("Sleep generator stopped yielding sleep values.") + time.sleep(next_sleep) class Retry(_BaseRetry): diff --git a/google/api_core/retry/retry_unary_async.py b/google/api_core/retry/retry_unary_async.py index e76a37bb..d9f9a25e 100644 --- a/google/api_core/retry/retry_unary_async.py +++ b/google/api_core/retry/retry_unary_async.py @@ -150,17 +150,17 @@ async def retry_target( deadline = time.monotonic() + timeout if timeout is not None else None error_list: list[Exception] = [] - for sleep in sleep_generator: + while True: try: return await target() # pylint: disable=broad-except # This function explicitly must deal with broad exceptions. except Exception as exc: # defer to shared logic for handling errors - _retry_error_helper( + next_sleep = _retry_error_helper( exc, deadline, - sleep, + sleep_generator, error_list, predicate, on_error, @@ -168,9 +168,7 @@ async def retry_target( timeout, ) # if exception not raised, sleep before next attempt - await asyncio.sleep(sleep) - - raise ValueError("Sleep generator stopped yielding sleep values.") + await asyncio.sleep(next_sleep) class AsyncRetry(_BaseRetry): From 240fed057e379a8375ebbf87cb0d554498be834f Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 13 Feb 2025 16:01:58 -0800 Subject: [PATCH 02/10] added comment --- google/api_core/retry/retry_base.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google/api_core/retry/retry_base.py b/google/api_core/retry/retry_base.py index 4244e362..9a2d1d39 100644 --- a/google/api_core/retry/retry_base.py +++ b/google/api_core/retry/retry_base.py @@ -214,6 +214,8 @@ def _retry_error_helper( raise final_exc from source_exc if on_error_fn is not None: on_error_fn(exc) + # next_sleep is fetched after the on_error callback to allow clients + # to update sleep_generator values dynamically in response to errors try: next_sleep = next(sleep_generator) except StopIteration: From 6710fcbbf821208281d89a6fa6d34457af089bea Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 13 Feb 2025 16:29:45 -0800 Subject: [PATCH 03/10] use single sleep iterator for retries --- google/api_core/retry/retry_base.py | 4 ++-- google/api_core/retry/retry_streaming.py | 3 ++- google/api_core/retry/retry_streaming_async.py | 3 ++- google/api_core/retry/retry_unary.py | 3 ++- google/api_core/retry/retry_unary_async.py | 3 ++- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/google/api_core/retry/retry_base.py b/google/api_core/retry/retry_base.py index 9a2d1d39..e90657c1 100644 --- a/google/api_core/retry/retry_base.py +++ b/google/api_core/retry/retry_base.py @@ -25,7 +25,7 @@ import time from enum import Enum -from typing import Any, Callable, Optional, Iterable, TYPE_CHECKING +from typing import Any, Callable, Optional, Iterator, TYPE_CHECKING import requests.exceptions @@ -174,7 +174,7 @@ def build_retry_error( def _retry_error_helper( exc: Exception, deadline: float | None, - sleep_generator: Iterable[float], + sleep_generator: Iterator[float], error_list: list[Exception], predicate_fn: Callable[[Exception], bool], on_error_fn: Callable[[Exception], None] | None, diff --git a/google/api_core/retry/retry_streaming.py b/google/api_core/retry/retry_streaming.py index 5711cbba..2045a941 100644 --- a/google/api_core/retry/retry_streaming.py +++ b/google/api_core/retry/retry_streaming.py @@ -107,6 +107,7 @@ def retry_target_stream( time.monotonic() + timeout if timeout is not None else None ) error_list: list[Exception] = [] + sleep_iter = iter(sleep_generator) while True: # Start a new retry loop @@ -124,7 +125,7 @@ def retry_target_stream( next_sleep = _retry_error_helper( exc, deadline, - sleep_generator, + sleep_iter, error_list, predicate, on_error, diff --git a/google/api_core/retry/retry_streaming_async.py b/google/api_core/retry/retry_streaming_async.py index 2dc98179..9b5de1d2 100644 --- a/google/api_core/retry/retry_streaming_async.py +++ b/google/api_core/retry/retry_streaming_async.py @@ -109,6 +109,7 @@ async def retry_target_stream( deadline = time.monotonic() + timeout if timeout else None # keep track of retryable exceptions we encounter to pass in to exception_factory error_list: list[Exception] = [] + sleep_iter = iter(sleep_generator) target_is_generator: bool | None = None while True: @@ -177,7 +178,7 @@ async def retry_target_stream( next_sleep = _retry_error_helper( exc, deadline, - sleep_generator, + sleep_iter, error_list, predicate, on_error, diff --git a/google/api_core/retry/retry_unary.py b/google/api_core/retry/retry_unary.py index 86552004..dc2544b2 100644 --- a/google/api_core/retry/retry_unary.py +++ b/google/api_core/retry/retry_unary.py @@ -138,6 +138,7 @@ def retry_target( deadline = time.monotonic() + timeout if timeout is not None else None error_list: list[Exception] = [] + sleep_iter = iter(sleep_generator) while True: try: @@ -153,7 +154,7 @@ def retry_target( next_sleep = _retry_error_helper( exc, deadline, - sleep_generator, + sleep_iter, error_list, predicate, on_error, diff --git a/google/api_core/retry/retry_unary_async.py b/google/api_core/retry/retry_unary_async.py index d9f9a25e..f515b348 100644 --- a/google/api_core/retry/retry_unary_async.py +++ b/google/api_core/retry/retry_unary_async.py @@ -149,6 +149,7 @@ async def retry_target( deadline = time.monotonic() + timeout if timeout is not None else None error_list: list[Exception] = [] + sleep_iter = iter(sleep_generator) while True: try: @@ -160,7 +161,7 @@ async def retry_target( next_sleep = _retry_error_helper( exc, deadline, - sleep_generator, + sleep_iter, error_list, predicate, on_error, From d99844a5bf57affeca8cc47fc2de62992937f4da Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 13 Feb 2025 16:30:05 -0800 Subject: [PATCH 04/10] fix tests --- tests/asyncio/retry/test_retry_streaming_async.py | 2 +- tests/asyncio/retry/test_retry_unary_async.py | 2 +- tests/unit/retry/test_retry_streaming.py | 2 +- tests/unit/retry/test_retry_unary.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/asyncio/retry/test_retry_streaming_async.py b/tests/asyncio/retry/test_retry_streaming_async.py index d0fd799c..4ce3c927 100644 --- a/tests/asyncio/retry/test_retry_streaming_async.py +++ b/tests/asyncio/retry/test_retry_streaming_async.py @@ -36,7 +36,7 @@ async def test_retry_streaming_target_bad_sleep_generator(): from google.api_core.retry.retry_streaming_async import retry_target_stream with pytest.raises(ValueError, match="Sleep generator"): - await retry_target_stream(None, None, [], None).__anext__() + await retry_target_stream(None, lambda x: True, [], None).__anext__() class TestAsyncStreamingRetry(Test_BaseRetry): diff --git a/tests/asyncio/retry/test_retry_unary_async.py b/tests/asyncio/retry/test_retry_unary_async.py index dc64299f..2a51dd8d 100644 --- a/tests/asyncio/retry/test_retry_unary_async.py +++ b/tests/asyncio/retry/test_retry_unary_async.py @@ -137,7 +137,7 @@ async def test_retry_target_timeout_exceeded(monotonic, sleep, use_deadline_arg) async def test_retry_target_bad_sleep_generator(): with pytest.raises(ValueError, match="Sleep generator"): await retry_async.retry_target( - mock.sentinel.target, mock.sentinel.predicate, [], None + mock.sentinel.target, lambda x: True, [], None ) diff --git a/tests/unit/retry/test_retry_streaming.py b/tests/unit/retry/test_retry_streaming.py index 0bc85d92..c21043da 100644 --- a/tests/unit/retry/test_retry_streaming.py +++ b/tests/unit/retry/test_retry_streaming.py @@ -33,7 +33,7 @@ def test_retry_streaming_target_bad_sleep_generator(): with pytest.raises( ValueError, match="Sleep generator stopped yielding sleep values" ): - next(retry_streaming.retry_target_stream(None, None, [], None)) + next(retry_streaming.retry_target_stream(None, lambda x: True, [], None)) class TestStreamingRetry(Test_BaseRetry): diff --git a/tests/unit/retry/test_retry_unary.py b/tests/unit/retry/test_retry_unary.py index 6851fbe4..035fc8f2 100644 --- a/tests/unit/retry/test_retry_unary.py +++ b/tests/unit/retry/test_retry_unary.py @@ -146,7 +146,7 @@ def test_retry_target_timeout_exceeded(monotonic, sleep, use_deadline_arg): def test_retry_target_bad_sleep_generator(): with pytest.raises(ValueError, match="Sleep generator"): - retry.retry_target(mock.sentinel.target, mock.sentinel.predicate, [], None) + retry.retry_target(mock.sentinel.target, lambda x: True, [], None) class TestRetry(Test_BaseRetry): From 465e023886a3eaa75c738646a47c12d514365e15 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 13 Feb 2025 16:34:36 -0800 Subject: [PATCH 05/10] update variable name --- google/api_core/retry/retry_base.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/google/api_core/retry/retry_base.py b/google/api_core/retry/retry_base.py index e90657c1..5bbc27aa 100644 --- a/google/api_core/retry/retry_base.py +++ b/google/api_core/retry/retry_base.py @@ -174,7 +174,7 @@ def build_retry_error( def _retry_error_helper( exc: Exception, deadline: float | None, - sleep_generator: Iterator[float], + sleep_iterator: Iterator[float], error_list: list[Exception], predicate_fn: Callable[[Exception], bool], on_error_fn: Callable[[Exception], None] | None, @@ -194,7 +194,7 @@ def _retry_error_helper( Args: - exc: the exception that was raised - deadline: the deadline for the retry, calculated as a diff from time.monotonic() - - sleep_generator: iterable to draw the next backoff value from + - sleep_iterator: iterator to draw the next backoff value from - error_list: the list of exceptions that have been raised so far - predicate_fn: takes `exc` and returns true if the operation should be retried - on_error_fn: callback to execute when a retryable error occurs @@ -214,10 +214,10 @@ def _retry_error_helper( raise final_exc from source_exc if on_error_fn is not None: on_error_fn(exc) - # next_sleep is fetched after the on_error callback to allow clients - # to update sleep_generator values dynamically in response to errors + # next_sleep is fetched after the on_error callback, to allow clients + # to update sleep_iterator values dynamically in response to errors try: - next_sleep = next(sleep_generator) + next_sleep = next(sleep_iterator) except StopIteration: raise ValueError("Sleep generator stopped yielding sleep values.") from exc if deadline is not None and time.monotonic() + next_sleep > deadline: From 7e7c078a50c165998f073ad60a8a9bb2733c13b9 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 13 Feb 2025 16:35:47 -0800 Subject: [PATCH 06/10] adjusted docstring --- google/api_core/retry/retry_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/retry/retry_base.py b/google/api_core/retry/retry_base.py index 5bbc27aa..263b4ccf 100644 --- a/google/api_core/retry/retry_base.py +++ b/google/api_core/retry/retry_base.py @@ -202,7 +202,7 @@ def _retry_error_helper( - original_timeout_val: the original timeout value for the retry (in seconds), to be passed to the exception factory for building an error message Returns: - - the next backoff value to use + - the sleep value chosen before the next attempt """ error_list.append(exc) if not predicate_fn(exc): From 7498f2b6e1ec75d06c5db6585849d3139fac72f9 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 14 Feb 2025 00:38:06 +0000 Subject: [PATCH 07/10] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- .kokoro/docker/docs/requirements.txt | 42 +++++++++---------- tests/asyncio/retry/test_retry_unary_async.py | 4 +- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/.kokoro/docker/docs/requirements.txt b/.kokoro/docker/docs/requirements.txt index d6e93894..a9360a25 100644 --- a/.kokoro/docker/docs/requirements.txt +++ b/.kokoro/docker/docs/requirements.txt @@ -8,9 +8,9 @@ argcomplete==3.5.3 \ --hash=sha256:2ab2c4a215c59fd6caaff41a869480a23e8f6a5f910b266c1808037f4e375b61 \ --hash=sha256:c12bf50eded8aebb298c7b7da7a5ff3ee24dffd9f5281867dfe1424b58c55392 # via nox -cachetools==5.5.1 \ - --hash=sha256:70f238fbba50383ef62e55c6aff6d9673175fe59f7c6782c7a0b9e38f4a9df95 \ - --hash=sha256:b76651fdc3b24ead3c648bbdeeb940c1b04d365b38b4af66788f9ec4a81d42bb +cachetools==5.5.0 \ + --hash=sha256:02134e8439cdc2ffb62023ce1debca2944c3f289d66bb17ead3ab3dede74b292 \ + --hash=sha256:2cc24fb4cbe39633fb7badd9db9ca6295d766d9c2995f245725a46715d050f2a # via google-auth certifi==2024.12.14 \ --hash=sha256:1275f7a45be9464efc1173084eaa30f866fe2e47d389406136d332ed4967ec56 \ @@ -124,23 +124,23 @@ distlib==0.3.9 \ --hash=sha256:47f8c22fd27c27e25a65601af709b38e4f0a45ea4fc2e710f65755fa8caaaf87 \ --hash=sha256:a60f20dea646b8a33f3e7772f74dc0b2d0772d2837ee1342a00645c81edf9403 # via virtualenv -filelock==3.17.0 \ - --hash=sha256:533dc2f7ba78dc2f0f531fc6c4940addf7b70a481e269a5a3b93be94ffbe8338 \ - --hash=sha256:ee4e77401ef576ebb38cd7f13b9b28893194acc20a8e68e18730ba9c0e54660e +filelock==3.16.1 \ + --hash=sha256:2082e5703d51fbf98ea75855d9d5527e33d8ff23099bec374a134febee6946b0 \ + --hash=sha256:c249fbfcd5db47e5e2d6d62198e565475ee65e4831e2561c8e313fa7eb961435 # via virtualenv gcp-docuploader==0.6.5 \ --hash=sha256:30221d4ac3e5a2b9c69aa52fdbef68cc3f27d0e6d0d90e220fc024584b8d2318 \ --hash=sha256:b7458ef93f605b9d46a4bf3a8dc1755dad1f31d030c8679edf304e343b347eea # via -r requirements.in -google-api-core==2.24.1 \ - --hash=sha256:bc78d608f5a5bf853b80bd70a795f703294de656c096c0968320830a4bc280f1 \ - --hash=sha256:f8b36f5456ab0dd99a1b693a40a31d1e7757beea380ad1b38faaf8941eae9d8a +google-api-core==2.24.0 \ + --hash=sha256:10d82ac0fca69c82a25b3efdeefccf6f28e02ebb97925a8cce8edbfe379929d9 \ + --hash=sha256:e255640547a597a4da010876d333208ddac417d60add22b6851a0c66a831fcaf # via # google-cloud-core # google-cloud-storage -google-auth==2.38.0 \ - --hash=sha256:8285113607d3b80a3f1543b75962447ba8a09fe85783432a784fdeef6ac094c4 \ - --hash=sha256:e7dae6694313f434a2727bf2906f27ad259bae090d7aa896590d86feec3d9d4a +google-auth==2.37.0 \ + --hash=sha256:0054623abf1f9c83492c63d3f47e77f0a544caa3d40b2d98e099a611c2dd5d00 \ + --hash=sha256:42664f18290a6be591be5329a96fe30184be1a1badb7292a7f686a9659de9ca0 # via # google-api-core # google-cloud-core @@ -149,9 +149,9 @@ google-cloud-core==2.4.1 \ --hash=sha256:9b7749272a812bde58fff28868d0c5e2f585b82f37e09a1f6ed2d4d10f134073 \ --hash=sha256:a9e6a4422b9ac5c29f79a0ede9485473338e2ce78d91f2370c01e730eab22e61 # via google-cloud-storage -google-cloud-storage==3.0.0 \ - --hash=sha256:2accb3e828e584888beff1165e5f3ac61aa9088965eb0165794a82d8c7f95297 \ - --hash=sha256:f85fd059650d2dbb0ac158a9a6b304b66143b35ed2419afec2905ca522eb2c6a +google-cloud-storage==2.19.0 \ + --hash=sha256:aeb971b5c29cf8ab98445082cbfe7b161a1f48ed275822f59ed3f1524ea54fba \ + --hash=sha256:cd05e9e7191ba6cb68934d8eb76054d9be4562aa89dbc4236feee4d7d51342b2 # via gcp-docuploader google-crc32c==1.6.0 \ --hash=sha256:05e2d8c9a2f853ff116db9706b4a27350587f341eda835f46db3c0a8c8ce2f24 \ @@ -208,9 +208,9 @@ platformdirs==4.3.6 \ --hash=sha256:357fb2acbc885b0419afd3ce3ed34564c13c9b95c89360cd9563f73aa5e2b907 \ --hash=sha256:73e575e1408ab8103900836b97580d5307456908a03e92031bab39e4554cc3fb # via virtualenv -proto-plus==1.26.0 \ - --hash=sha256:6e93d5f5ca267b54300880fff156b6a3386b3fa3f43b1da62e680fc0c586ef22 \ - --hash=sha256:bf2dfaa3da281fc3187d12d224c707cb57214fb2c22ba854eb0c105a3fb2d4d7 +proto-plus==1.25.0 \ + --hash=sha256:c91fc4a65074ade8e458e95ef8bac34d4008daa7cce4a12d6707066fca648961 \ + --hash=sha256:fbb17f57f7bd05a68b7707e745e26528b0b3c34e378db91eef93912c54982d91 # via google-api-core protobuf==5.29.3 \ --hash=sha256:0a18ed4a24198528f2333802eb075e59dea9d679ab7a6c5efb017a59004d849f \ @@ -291,7 +291,7 @@ urllib3==2.3.0 \ --hash=sha256:1cee9ad369867bfdbbb48b7dd50374c0967a0bb7710050facf0dd6911440e3df \ --hash=sha256:f8c5449b3cf0861679ce7e0503c7b44b5ec981bec0d1d3795a07f1ba96f0204d # via requests -virtualenv==20.29.1 \ - --hash=sha256:4e4cb403c0b0da39e13b46b1b2476e505cb0046b25f242bee80f62bf990b2779 \ - --hash=sha256:b8b8970138d32fb606192cb97f6cd4bb644fa486be9308fb9b63f81091b5dc35 +virtualenv==20.28.1 \ + --hash=sha256:412773c85d4dab0409b83ec36f7a6499e72eaf08c80e81e9576bca61831c71cb \ + --hash=sha256:5d34ab240fdb5d21549b76f9e8ff3af28252f5499fb6d6f031adac4e5a8c5329 # via nox diff --git a/tests/asyncio/retry/test_retry_unary_async.py b/tests/asyncio/retry/test_retry_unary_async.py index 2a51dd8d..75d1f090 100644 --- a/tests/asyncio/retry/test_retry_unary_async.py +++ b/tests/asyncio/retry/test_retry_unary_async.py @@ -136,9 +136,7 @@ async def test_retry_target_timeout_exceeded(monotonic, sleep, use_deadline_arg) @pytest.mark.asyncio async def test_retry_target_bad_sleep_generator(): with pytest.raises(ValueError, match="Sleep generator"): - await retry_async.retry_target( - mock.sentinel.target, lambda x: True, [], None - ) + await retry_async.retry_target(mock.sentinel.target, lambda x: True, [], None) class TestAsyncRetry(Test_BaseRetry): From 374f17ea6c12f13c39d65e54e16622a2adaf1ad0 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 14 Feb 2025 09:43:36 -0800 Subject: [PATCH 08/10] fixed mypy issues --- google/api_core/retry/retry_streaming.py | 4 ++-- google/api_core/retry/retry_streaming_async.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google/api_core/retry/retry_streaming.py b/google/api_core/retry/retry_streaming.py index 2045a941..1c1969f3 100644 --- a/google/api_core/retry/retry_streaming.py +++ b/google/api_core/retry/retry_streaming.py @@ -59,8 +59,8 @@ def retry_target_stream( [List[Exception], RetryFailureReason, Optional[float]], Tuple[Exception, Optional[Exception]], ] = build_retry_error, - init_args: _P.args = (), - init_kwargs: _P.kwargs = {}, + init_args: tuple = (), + init_kwargs: dict = {}, **kwargs, ) -> Generator[_Y, Any, None]: """Create a generator wrapper that retries the wrapped stream if it fails. diff --git a/google/api_core/retry/retry_streaming_async.py b/google/api_core/retry/retry_streaming_async.py index 9b5de1d2..830c1d0d 100644 --- a/google/api_core/retry/retry_streaming_async.py +++ b/google/api_core/retry/retry_streaming_async.py @@ -62,8 +62,8 @@ async def retry_target_stream( [list[Exception], RetryFailureReason, float | None], tuple[Exception, Exception | None], ] = build_retry_error, - init_args: _P.args = (), - init_kwargs: _P.kwargs = {}, + init_args: tuple = (), + init_kwargs: dict = {}, **kwargs, ) -> AsyncGenerator[_Y, None]: """Create a generator wrapper that retries the wrapped stream if it fails. From eb74572ff94faf0a61955daafd64578caeef2af2 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 12 Mar 2025 17:02:30 -0700 Subject: [PATCH 09/10] added comment --- google/api_core/retry/retry_streaming.py | 2 ++ google/api_core/retry/retry_streaming_async.py | 2 ++ google/api_core/retry/retry_unary.py | 2 ++ google/api_core/retry/retry_unary_async.py | 2 ++ 4 files changed, 8 insertions(+) diff --git a/google/api_core/retry/retry_streaming.py b/google/api_core/retry/retry_streaming.py index 1c1969f3..e4474c8a 100644 --- a/google/api_core/retry/retry_streaming.py +++ b/google/api_core/retry/retry_streaming.py @@ -109,6 +109,8 @@ def retry_target_stream( error_list: list[Exception] = [] sleep_iter = iter(sleep_generator) + # continue trying until an attempt completes, or a terminal exception is raised in _retry_error_helper + # TODO: support max_attempts argument: https://github.com/googleapis/python-api-core/issues/535 while True: # Start a new retry loop try: diff --git a/google/api_core/retry/retry_streaming_async.py b/google/api_core/retry/retry_streaming_async.py index 830c1d0d..5e5fa240 100644 --- a/google/api_core/retry/retry_streaming_async.py +++ b/google/api_core/retry/retry_streaming_async.py @@ -112,6 +112,8 @@ async def retry_target_stream( sleep_iter = iter(sleep_generator) target_is_generator: bool | None = None + # continue trying until an attempt completes, or a terminal exception is raised in _retry_error_helper + # TODO: support max_attempts argument: https://github.com/googleapis/python-api-core/issues/535 while True: # Start a new retry loop try: diff --git a/google/api_core/retry/retry_unary.py b/google/api_core/retry/retry_unary.py index dc2544b2..6d36bc7d 100644 --- a/google/api_core/retry/retry_unary.py +++ b/google/api_core/retry/retry_unary.py @@ -140,6 +140,8 @@ def retry_target( error_list: list[Exception] = [] sleep_iter = iter(sleep_generator) + # continue trying until an attempt completes, or a terminal exception is raised in _retry_error_helper + # TODO: support max_attempts argument: https://github.com/googleapis/python-api-core/issues/535 while True: try: result = target() diff --git a/google/api_core/retry/retry_unary_async.py b/google/api_core/retry/retry_unary_async.py index f515b348..1f72476a 100644 --- a/google/api_core/retry/retry_unary_async.py +++ b/google/api_core/retry/retry_unary_async.py @@ -151,6 +151,8 @@ async def retry_target( error_list: list[Exception] = [] sleep_iter = iter(sleep_generator) + # continue trying until an attempt completes, or a terminal exception is raised in _retry_error_helper + # TODO: support max_attempts argument: https://github.com/googleapis/python-api-core/issues/535 while True: try: return await target() From e9940f3a66dc02206a109967fc178d86e14370bf Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 17 Mar 2025 16:21:47 -0700 Subject: [PATCH 10/10] added unit tests for dynamic backoff --- .../retry/test_retry_streaming_async.py | 33 +++++++++++++++++-- tests/asyncio/retry/test_retry_unary_async.py | 27 +++++++++++++++ tests/unit/retry/test_retry_streaming.py | 33 +++++++++++++++++-- tests/unit/retry/test_retry_unary.py | 26 +++++++++++++++ 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/tests/asyncio/retry/test_retry_streaming_async.py b/tests/asyncio/retry/test_retry_streaming_async.py index 4ce3c927..e44f5361 100644 --- a/tests/asyncio/retry/test_retry_streaming_async.py +++ b/tests/asyncio/retry/test_retry_streaming_async.py @@ -39,6 +39,35 @@ async def test_retry_streaming_target_bad_sleep_generator(): await retry_target_stream(None, lambda x: True, [], None).__anext__() +@mock.patch("asyncio.sleep", autospec=True) +@pytest.mark.asyncio +async def test_retry_streaming_target_dynamic_backoff(sleep): + """ + sleep_generator should be iterated after on_error, to support dynamic backoff + """ + from functools import partial + from google.api_core.retry.retry_streaming_async import retry_target_stream + + sleep.side_effect = RuntimeError("stop after sleep") + # start with empty sleep generator; values are added after exception in push_sleep_value + sleep_values = [] + error_target = partial(TestAsyncStreamingRetry._generator_mock, error_on=0) + inserted_sleep = 99 + + def push_sleep_value(err): + sleep_values.append(inserted_sleep) + + with pytest.raises(RuntimeError): + await retry_target_stream( + error_target, + predicate=lambda x: True, + sleep_generator=sleep_values, + on_error=push_sleep_value, + ).__anext__() + assert sleep.call_count == 1 + sleep.assert_called_once_with(inserted_sleep) + + class TestAsyncStreamingRetry(Test_BaseRetry): def _make_one(self, *args, **kwargs): return retry_streaming_async.AsyncStreamingRetry(*args, **kwargs) @@ -66,8 +95,8 @@ def if_exception_type(exc): str(retry_), ) + @staticmethod async def _generator_mock( - self, num=5, error_on=None, exceptions_seen=None, @@ -87,7 +116,7 @@ async def _generator_mock( for i in range(num): if sleep_time: await asyncio.sleep(sleep_time) - if error_on and i == error_on: + if error_on is not None and i == error_on: raise ValueError("generator mock error") yield i except (Exception, BaseException, GeneratorExit) as e: diff --git a/tests/asyncio/retry/test_retry_unary_async.py b/tests/asyncio/retry/test_retry_unary_async.py index 75d1f090..e7fdc963 100644 --- a/tests/asyncio/retry/test_retry_unary_async.py +++ b/tests/asyncio/retry/test_retry_unary_async.py @@ -139,6 +139,33 @@ async def test_retry_target_bad_sleep_generator(): await retry_async.retry_target(mock.sentinel.target, lambda x: True, [], None) +@mock.patch("asyncio.sleep", autospec=True) +@pytest.mark.asyncio +async def test_retry_target_dynamic_backoff(sleep): + """ + sleep_generator should be iterated after on_error, to support dynamic backoff + """ + sleep.side_effect = RuntimeError("stop after sleep") + # start with empty sleep generator; values are added after exception in push_sleep_value + sleep_values = [] + exception = ValueError("trigger retry") + error_target = mock.Mock(side_effect=exception) + inserted_sleep = 99 + + def push_sleep_value(err): + sleep_values.append(inserted_sleep) + + with pytest.raises(RuntimeError): + await retry_async.retry_target( + error_target, + predicate=lambda x: True, + sleep_generator=sleep_values, + on_error=push_sleep_value, + ) + assert sleep.call_count == 1 + sleep.assert_called_once_with(inserted_sleep) + + class TestAsyncRetry(Test_BaseRetry): def _make_one(self, *args, **kwargs): return retry_async.AsyncRetry(*args, **kwargs) diff --git a/tests/unit/retry/test_retry_streaming.py b/tests/unit/retry/test_retry_streaming.py index c21043da..2499b2ae 100644 --- a/tests/unit/retry/test_retry_streaming.py +++ b/tests/unit/retry/test_retry_streaming.py @@ -36,6 +36,35 @@ def test_retry_streaming_target_bad_sleep_generator(): next(retry_streaming.retry_target_stream(None, lambda x: True, [], None)) +@mock.patch("time.sleep", autospec=True) +def test_retry_streaming_target_dynamic_backoff(sleep): + """ + sleep_generator should be iterated after on_error, to support dynamic backoff + """ + from functools import partial + + sleep.side_effect = RuntimeError("stop after sleep") + # start with empty sleep generator; values are added after exception in push_sleep_value + sleep_values = [] + error_target = partial(TestStreamingRetry._generator_mock, error_on=0) + inserted_sleep = 99 + + def push_sleep_value(err): + sleep_values.append(inserted_sleep) + + with pytest.raises(RuntimeError): + next( + retry_streaming.retry_target_stream( + error_target, + predicate=lambda x: True, + sleep_generator=sleep_values, + on_error=push_sleep_value, + ) + ) + assert sleep.call_count == 1 + sleep.assert_called_once_with(inserted_sleep) + + class TestStreamingRetry(Test_BaseRetry): def _make_one(self, *args, **kwargs): return retry_streaming.StreamingRetry(*args, **kwargs) @@ -63,8 +92,8 @@ def if_exception_type(exc): str(retry_), ) + @staticmethod def _generator_mock( - self, num=5, error_on=None, return_val=None, @@ -82,7 +111,7 @@ def _generator_mock( """ try: for i in range(num): - if error_on and i == error_on: + if error_on is not None and i == error_on: raise ValueError("generator mock error") yield i return return_val diff --git a/tests/unit/retry/test_retry_unary.py b/tests/unit/retry/test_retry_unary.py index 035fc8f2..f5bbcff7 100644 --- a/tests/unit/retry/test_retry_unary.py +++ b/tests/unit/retry/test_retry_unary.py @@ -149,6 +149,32 @@ def test_retry_target_bad_sleep_generator(): retry.retry_target(mock.sentinel.target, lambda x: True, [], None) +@mock.patch("time.sleep", autospec=True) +def test_retry_target_dynamic_backoff(sleep): + """ + sleep_generator should be iterated after on_error, to support dynamic backoff + """ + sleep.side_effect = RuntimeError("stop after sleep") + # start with empty sleep generator; values are added after exception in push_sleep_value + sleep_values = [] + exception = ValueError("trigger retry") + error_target = mock.Mock(side_effect=exception) + inserted_sleep = 99 + + def push_sleep_value(err): + sleep_values.append(inserted_sleep) + + with pytest.raises(RuntimeError): + retry.retry_target( + error_target, + predicate=lambda x: True, + sleep_generator=sleep_values, + on_error=push_sleep_value, + ) + assert sleep.call_count == 1 + sleep.assert_called_once_with(inserted_sleep) + + class TestRetry(Test_BaseRetry): def _make_one(self, *args, **kwargs): return retry.Retry(*args, **kwargs)