Skip to content

Commit 5760b0d

Browse files
GallaecioGallaecio
authored
Make the default retry policy the aggressive one with half the attempts (#78)
1 parent d184f51 commit 5760b0d

File tree

4 files changed

+116
-170
lines changed

4 files changed

+116
-170
lines changed

‎CHANGES.rst‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,19 @@
11
Changes
22
=======
33

4+
Unreleased
5+
----------
6+
7+
* **Backward-incompatible:** Renamed some methods of
8+
:class:`~.RetryFactory` for consistency, since they now handle both temporary
9+
and permanent download errors:
10+
11+
* ``temporary_download_error_stop`` →
12+
:meth:`~.RetryFactory.download_error_stop`
13+
14+
* ``temporary_download_error_wait`` →
15+
:meth:`~.RetryFactory.download_error_wait`
16+
417
0.6.0 (2024-05-29)
518
------------------
619

‎docs/use/api.rst‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,32 +148,29 @@ retries for :ref:`rate-limiting <zapi-rate-limit>` and :ref:`unsuccessful
148148
.. _default-retry-policy:
149149

150150
The default retry policy, :data:`~zyte_api.zyte_api_retrying`, does the
151-
following:
151+
following for each request:
152152

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

155-
- Retries :ref:`temporary download errors
156-
<zapi-temporary-download-errors>` up to 3 times.
155+
- Retries :ref:`temporary download errors <zapi-temporary-download-errors>`
156+
up to 3 times. :ref:`Permanent download errors
157+
<zapi-permanent-download-errors>` also count towards this retry limit.
158+
159+
- Retries permanent download errors once.
157160

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

163+
- Retries error responses with an HTTP status code in the 500-599 range (503,
164+
520 and 521 excluded) once.
165+
160166
All retries are done with an exponential backoff algorithm.
161167

162168
.. _aggressive-retry-policy:
163169

164170
If some :ref:`unsuccessful responses <zapi-unsuccessful-responses>` exceed
165171
maximum retries with the default retry policy, try using
166-
:data:`~zyte_api.aggressive_retrying` instead, which modifies the default retry
167-
policy as follows:
168-
169-
- Temporary download error are retried 7 times. :ref:`Permanent download
170-
errors <zapi-permanent-download-errors>` also count towards this retry
171-
limit.
172-
173-
- Retries permanent download errors up to 3 times.
174-
175-
- Retries error responses with an HTTP status code in the 500-599 range (503,
176-
520 and 521 excluded) up to 3 times.
172+
:data:`~zyte_api.aggressive_retrying` instead, which doubles attempts for
173+
all retry scenarios.
177174

178175
Alternatively, the reference documentation of :class:`~zyte_api.RetryFactory`
179176
and :class:`~zyte_api.AggressiveRetryFactory` features some examples of custom

‎tests/test_retry.py‎

Lines changed: 39 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def broken_stop(_):
6666
("retry_factory", "status", "waiter"),
6767
[
6868
(RetryFactory, 429, "throttling"),
69-
(RetryFactory, 520, "temporary_download_error"),
69+
(RetryFactory, 520, "download_error"),
7070
(AggressiveRetryFactory, 429, "throttling"),
7171
(AggressiveRetryFactory, 500, "undocumented_error"),
7272
(AggressiveRetryFactory, 520, "download_error"),
@@ -140,6 +140,14 @@ def __init__(self, time):
140140
self.time = time
141141

142142

143+
class scale:
144+
def __init__(self, factor):
145+
self.factor = factor
146+
147+
def __call__(self, number, add=0):
148+
return int(number * self.factor) + add
149+
150+
143151
@pytest.mark.parametrize(
144152
("retrying", "outcomes", "exhausted"),
145153
[
@@ -237,81 +245,36 @@ def __init__(self, time):
237245
),
238246
)
239247
),
240-
# Behaviors specific to the default retry policy
248+
# Scaled behaviors, where the default retry policy uses half as many
249+
# attempts as the aggressive retry policy.
241250
*(
242-
(zyte_api_retrying, outcomes, exhausted)
243-
for outcomes, exhausted in (
244-
# Temporary download errors are retried until they have
245-
# happened 4 times in total.
246-
(
247-
(mock_request_error(status=520),) * 3,
248-
False,
249-
),
250-
(
251-
(mock_request_error(status=520),) * 4,
252-
True,
253-
),
254-
(
255-
(
256-
*(mock_request_error(status=429),) * 2,
257-
mock_request_error(status=520),
258-
),
259-
False,
260-
),
261-
(
262-
(
263-
*(mock_request_error(status=429),) * 3,
264-
mock_request_error(status=520),
265-
),
266-
False,
267-
),
268-
(
269-
(
270-
*(
271-
mock_request_error(status=429),
272-
mock_request_error(status=520),
273-
)
274-
* 3,
275-
),
276-
False,
277-
),
278-
(
279-
(
280-
*(
281-
mock_request_error(status=429),
282-
mock_request_error(status=520),
283-
)
284-
* 4,
285-
),
286-
True,
287-
),
251+
(retrying, outcomes, exhausted)
252+
for retrying, scaled in (
253+
(zyte_api_retrying, scale(0.5)),
254+
(aggressive_retrying, scale(1)),
288255
)
289-
),
290-
# Behaviors specific to the aggressive retry policy
291-
*(
292-
(aggressive_retrying, outcomes, exhausted)
293256
for outcomes, exhausted in (
294257
# Temporary download errors are retried until they have
295-
# happened 8 times in total. Permanent download errors also
296-
# count towards that limit.
258+
# happened 8*factor times in total. Permanent download errors
259+
# also count towards that limit.
297260
(
298-
(mock_request_error(status=520),) * 7,
261+
(mock_request_error(status=520),) * scaled(8, -1),
299262
False,
300263
),
301264
(
302-
(mock_request_error(status=520),) * 8,
265+
(mock_request_error(status=520),) * scaled(8),
303266
True,
304267
),
305268
(
306269
(
307-
*(mock_request_error(status=429),) * 6,
270+
*(mock_request_error(status=429),) * scaled(8, -2),
308271
mock_request_error(status=520),
309272
),
310273
False,
311274
),
312275
(
313276
(
314-
*(mock_request_error(status=429),) * 7,
277+
*(mock_request_error(status=429),) * scaled(8, -1),
315278
mock_request_error(status=520),
316279
),
317280
False,
@@ -322,7 +285,7 @@ def __init__(self, time):
322285
mock_request_error(status=429),
323286
mock_request_error(status=520),
324287
)
325-
* 7,
288+
* scaled(8, -1),
326289
),
327290
False,
328291
),
@@ -332,51 +295,52 @@ def __init__(self, time):
332295
mock_request_error(status=429),
333296
mock_request_error(status=520),
334297
)
335-
* 8,
298+
* scaled(8),
336299
),
337300
True,
338301
),
339302
(
340303
(
341-
*(mock_request_error(status=520),) * 5,
304+
*(mock_request_error(status=520),) * scaled(8, -3),
342305
*(mock_request_error(status=521),) * 1,
343306
*(mock_request_error(status=520),) * 1,
344307
),
345308
False,
346309
),
347310
(
348311
(
349-
*(mock_request_error(status=520),) * 6,
312+
*(mock_request_error(status=520),) * scaled(8, -2),
350313
*(mock_request_error(status=521),) * 1,
351314
*(mock_request_error(status=520),) * 1,
352315
),
353316
True,
354317
),
355318
(
356319
(
357-
*(mock_request_error(status=520),) * 6,
320+
*(mock_request_error(status=520),) * scaled(8, -2),
358321
*(mock_request_error(status=521),) * 1,
359322
),
360323
False,
361324
),
362325
(
363326
(
364-
*(mock_request_error(status=520),) * 7,
327+
*(mock_request_error(status=520),) * scaled(8, -1),
365328
*(mock_request_error(status=521),) * 1,
366329
),
367330
True,
368331
),
369332
# Permanent download errors are retried until they have
370-
# happened 4 times in total.
333+
# happened 4*factor times in total.
371334
(
372-
(*(mock_request_error(status=521),) * 3,),
335+
(*(mock_request_error(status=521),) * scaled(4, -1),),
373336
False,
374337
),
375338
(
376-
(*(mock_request_error(status=521),) * 4,),
339+
(*(mock_request_error(status=521),) * scaled(4),),
377340
True,
378341
),
379-
# Undocumented 5xx errors are retried up to 3 times.
342+
# Undocumented 5xx errors are retried until they have happened
343+
# 4*factor times.
380344
*(
381345
scenario
382346
for status in (
@@ -386,16 +350,16 @@ def __init__(self, time):
386350
)
387351
for scenario in (
388352
(
389-
(*(mock_request_error(status=status),) * 3,),
353+
(*(mock_request_error(status=status),) * scaled(4, -1),),
390354
False,
391355
),
392356
(
393-
(*(mock_request_error(status=status),) * 4,),
357+
(*(mock_request_error(status=status),) * scaled(4),),
394358
True,
395359
),
396360
(
397361
(
398-
*(mock_request_error(status=status),) * 2,
362+
*(mock_request_error(status=status),) * scaled(4, -2),
399363
mock_request_error(status=429),
400364
mock_request_error(status=503),
401365
ServerConnectionError(),
@@ -405,7 +369,7 @@ def __init__(self, time):
405369
),
406370
(
407371
(
408-
*(mock_request_error(status=status),) * 3,
372+
*(mock_request_error(status=status),) * scaled(4, -1),
409373
mock_request_error(status=429),
410374
mock_request_error(status=503),
411375
ServerConnectionError(),
@@ -415,17 +379,15 @@ def __init__(self, time):
415379
),
416380
(
417381
(
418-
mock_request_error(status=status),
419382
mock_request_error(status=555),
420-
mock_request_error(status=status),
383+
*(mock_request_error(status=status),) * scaled(4, -2),
421384
),
422385
False,
423386
),
424387
(
425388
(
426-
mock_request_error(status=status),
427389
mock_request_error(status=555),
428-
*(mock_request_error(status=status),) * 2,
390+
*(mock_request_error(status=status),) * scaled(4, -1),
429391
),
430392
True,
431393
),
@@ -464,7 +426,7 @@ async def run():
464426
try:
465427
await run()
466428
except Exception as outcome:
467-
assert exhausted
429+
assert exhausted, outcome # noqa: PT017
468430
assert outcome is last_outcome # noqa: PT017
469431
else:
470432
assert not exhausted

0 commit comments

Comments
 (0)