Skip to content

Make the default retry policy the aggressive one with half the attempts #78

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.

Sign up for GitHub

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

Merged
Gallaecio merged 8 commits into from
Feb 16, 2025
Merged

Make the default retry policy the aggressive one with half the attempts #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d60a9a
Make the default retry policy the aggressive one with half the attempts
Gallaecio Dec 30, 2024
d816d4a
Update docs/use/api.rst
Gallaecio Dec 31, 2024
26f21d0
Update api.rst
Gallaecio Dec 31, 2024
1d6645e
Update api.rst
Gallaecio Dec 31, 2024
6984f64
Merge remote-tracking branch 'zytedata/main' into retry-5xx
Gallaecio Feb 16, 2025
d966adb
Remove the temporary_ prefix from methods that now also handle perman…
Gallaecio Feb 16, 2025
7cd5dcb
Merge remote-tracking branch 'Gallaecio/retry-5xx' into retry-5xx
Gallaecio Feb 16, 2025
4bacd05
Restore mypy skips
Gallaecio Feb 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
Changes
=======

Unreleased
----------

* **Backward-incompatible:** Renamed some methods of
:class:`~.RetryFactory` for consistency, since they now handle both temporary
and permanent download errors:

* ``temporary_download_error_stop`` →
:meth:`~.RetryFactory.download_error_stop`

* ``temporary_download_error_wait`` →
:meth:`~.RetryFactory.download_error_wait`

0.6.0 (2024-05-29)
------------------

Expand Down
25 changes: 11 additions & 14 deletions docs/use/api.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,32 +148,29 @@ retries for :ref:`rate-limiting <zapi-rate-limit>` and :ref:`unsuccessful
.. _default-retry-policy:

The default retry policy, :data:`~zyte_api.zyte_api_retrying`, does the
following:
following for each request:

- Retries :ref:`rate-limiting responses <zapi-rate-limit>` forever.

- Retries :ref:`temporary download errors
<zapi-temporary-download-errors>` up to 3 times.
- Retries :ref:`temporary download errors <zapi-temporary-download-errors>`
up to 3 times. :ref:`Permanent download errors
<zapi-permanent-download-errors>` also count towards this retry limit.

- Retries permanent download errors once.

- Retries network errors until they have happened for 15 minutes straight.

- Retries error responses with an HTTP status code in the 500-599 range (503,
520 and 521 excluded) once.

All retries are done with an exponential backoff algorithm.

.. _aggressive-retry-policy:

If some :ref:`unsuccessful responses <zapi-unsuccessful-responses>` exceed
maximum retries with the default retry policy, try using
:data:`~zyte_api.aggressive_retrying` instead, which modifies the default retry
policy as follows:

- Temporary download error are retried 7 times. :ref:`Permanent download
errors <zapi-permanent-download-errors>` also count towards this retry
limit.

- Retries permanent download errors up to 3 times.

- Retries error responses with an HTTP status code in the 500-599 range (503,
520 and 521 excluded) up to 3 times.
:data:`~zyte_api.aggressive_retrying` instead, which doubles attempts for
all retry scenarios.

Alternatively, the reference documentation of :class:`~zyte_api.RetryFactory`
and :class:`~zyte_api.AggressiveRetryFactory` features some examples of custom
Expand Down
116 changes: 39 additions & 77 deletions tests/test_retry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def broken_stop(_):
("retry_factory", "status", "waiter"),
[
(RetryFactory, 429, "throttling"),
(RetryFactory, 520, "temporary_download_error"),
(RetryFactory, 520, "download_error"),
(AggressiveRetryFactory, 429, "throttling"),
(AggressiveRetryFactory, 500, "undocumented_error"),
(AggressiveRetryFactory, 520, "download_error"),
Expand Down Expand Up @@ -140,6 +140,14 @@ def __init__(self, time):
self.time = time


class scale:
def __init__(self, factor):
self.factor = factor

def __call__(self, number, add=0):
return int(number * self.factor) + add


@pytest.mark.parametrize(
("retrying", "outcomes", "exhausted"),
[
Expand Down Expand Up @@ -237,81 +245,36 @@ def __init__(self, time):
),
)
),
# Behaviors specific to the default retry policy
# Scaled behaviors, where the default retry policy uses half as many
# attempts as the aggressive retry policy.
*(
(zyte_api_retrying, outcomes, exhausted)
for outcomes, exhausted in (
# Temporary download errors are retried until they have
# happened 4 times in total.
(
(mock_request_error(status=520),) * 3,
False,
),
(
(mock_request_error(status=520),) * 4,
True,
),
(
(
*(mock_request_error(status=429),) * 2,
mock_request_error(status=520),
),
False,
),
(
(
*(mock_request_error(status=429),) * 3,
mock_request_error(status=520),
),
False,
),
(
(
*(
mock_request_error(status=429),
mock_request_error(status=520),
)
* 3,
),
False,
),
(
(
*(
mock_request_error(status=429),
mock_request_error(status=520),
)
* 4,
),
True,
),
(retrying, outcomes, exhausted)
for retrying, scaled in (
(zyte_api_retrying, scale(0.5)),
(aggressive_retrying, scale(1)),
)
),
# Behaviors specific to the aggressive retry policy
*(
(aggressive_retrying, outcomes, exhausted)
for outcomes, exhausted in (
# Temporary download errors are retried until they have
# happened 8 times in total. Permanent download errors also
# count towards that limit.
# happened 8*factor times in total. Permanent download errors
# also count towards that limit.
(
(mock_request_error(status=520),) * 7,
(mock_request_error(status=520),) * scaled(8, -1),
False,
),
(
(mock_request_error(status=520),) * 8,
(mock_request_error(status=520),) * scaled(8),
True,
),
(
(
*(mock_request_error(status=429),) * 6,
*(mock_request_error(status=429),) * scaled(8, -2),
mock_request_error(status=520),
),
False,
),
(
(
*(mock_request_error(status=429),) * 7,
*(mock_request_error(status=429),) * scaled(8, -1),
mock_request_error(status=520),
),
False,
Expand All @@ -322,7 +285,7 @@ def __init__(self, time):
mock_request_error(status=429),
mock_request_error(status=520),
)
* 7,
* scaled(8, -1),
),
False,
),
Expand All @@ -332,51 +295,52 @@ def __init__(self, time):
mock_request_error(status=429),
mock_request_error(status=520),
)
* 8,
* scaled(8),
),
True,
),
(
(
*(mock_request_error(status=520),) * 5,
*(mock_request_error(status=520),) * scaled(8, -3),
*(mock_request_error(status=521),) * 1,
*(mock_request_error(status=520),) * 1,
),
False,
),
(
(
*(mock_request_error(status=520),) * 6,
*(mock_request_error(status=520),) * scaled(8, -2),
*(mock_request_error(status=521),) * 1,
*(mock_request_error(status=520),) * 1,
),
True,
),
(
(
*(mock_request_error(status=520),) * 6,
*(mock_request_error(status=520),) * scaled(8, -2),
*(mock_request_error(status=521),) * 1,
),
False,
),
(
(
*(mock_request_error(status=520),) * 7,
*(mock_request_error(status=520),) * scaled(8, -1),
*(mock_request_error(status=521),) * 1,
),
True,
),
# Permanent download errors are retried until they have
# happened 4 times in total.
# happened 4*factor times in total.
(
(*(mock_request_error(status=521),) * 3,),
(*(mock_request_error(status=521),) * scaled(4, -1),),
False,
),
(
(*(mock_request_error(status=521),) * 4,),
(*(mock_request_error(status=521),) * scaled(4),),
True,
),
# Undocumented 5xx errors are retried up to 3 times.
# Undocumented 5xx errors are retried until they have happened
# 4*factor times.
*(
scenario
for status in (
Expand All @@ -386,16 +350,16 @@ def __init__(self, time):
)
for scenario in (
(
(*(mock_request_error(status=status),) * 3,),
(*(mock_request_error(status=status),) * scaled(4, -1),),
False,
),
(
(*(mock_request_error(status=status),) * 4,),
(*(mock_request_error(status=status),) * scaled(4),),
True,
),
(
(
*(mock_request_error(status=status),) * 2,
*(mock_request_error(status=status),) * scaled(4, -2),
mock_request_error(status=429),
mock_request_error(status=503),
ServerConnectionError(),
Expand All @@ -405,7 +369,7 @@ def __init__(self, time):
),
(
(
*(mock_request_error(status=status),) * 3,
*(mock_request_error(status=status),) * scaled(4, -1),
mock_request_error(status=429),
mock_request_error(status=503),
ServerConnectionError(),
Expand All @@ -415,17 +379,15 @@ def __init__(self, time):
),
(
(
mock_request_error(status=status),
mock_request_error(status=555),
mock_request_error(status=status),
*(mock_request_error(status=status),) * scaled(4, -2),
),
False,
),
(
(
mock_request_error(status=status),
mock_request_error(status=555),
*(mock_request_error(status=status),) * 2,
*(mock_request_error(status=status),) * scaled(4, -1),
),
True,
),
Expand Down Expand Up @@ -464,7 +426,7 @@ async def run():
try:
await run()
except Exception as outcome:
assert exhausted
assert exhausted, outcome # noqa: PT017
assert outcome is last_outcome # noqa: PT017
else:
assert not exhausted
Loading