Provide a @retry decorator to automatically retry Python functions in Checkbox jobs (new) by pieqq · Pull Request #1453 · canonical/checkbox

pieqq · 2024-09-04T15:10:04Z

Description

This PR does two things:

Implement a @retry decorator that can be used on any Python functions to automatically re-run it several times with an added delay/backoff/jitter when it raises an exception
Modify the networking_http.py script to make use of this

More info about the decorator:

When a function is decorated with @retry, it will be retried until it
passes or until the maximum defined attempts have been reached:

>>> from checkbox_support.helpers.retry import retry
>>> @retry(max_attempts=3, delay=10)
... def my_failing_test(arg):
...     return 1 / arg
... 
>>> result = my_failing_test(0)

===========
Attempt 1/3
===========
Attempt 1 failed:
division by zero

Waiting 3.61 seconds before retrying...

===========
Attempt 2/3
===========
Attempt 2 failed:
division by zero

Waiting 6.63 seconds before retrying...

===========
Attempt 3/3
===========
Attempt 3 failed:
division by zero

All the attempts have failed!
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/pieq/dev/work/checkbox/checkbox-support/checkbox_support/helpers/retry.py", line 89, in _f
    return run_with_retry(f, max_attempts, delay, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pieq/dev/work/checkbox/checkbox-support/checkbox_support/helpers/retry.py", line 56, in run_with_retry
    result = f(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^
  File "<stdin>", line 3, in my_failing_test
ZeroDivisionError: division by zero

>>> @retry(max_attempts=3, delay=10)
... def my_passing_test(arg):
...     return 10 * arg
... 

>>> result = my_passing_test(5)

===========
Attempt 1/3
===========
>>> result
50

Resolved issues

https://warthogs.atlassian.net/browse/CHECKBOX-1543

Note: This PR implements the decorator, but it does not modify the WiFi tests to make use of it. This will be done in a separate PR.

Documentation

Documented in the commit messages. Since this is part of checkbox-support, it is not documented in the readthedocs documentation.

Tests

Unit tests are provided, and I also ran it on the networking/http job in Checkbox:

The job works as before when everything is OK (it just adds an "Attempt 1/x" at the beginning of the outcome):

$ TRANSFER_SERVER="ubuntu.com" checkbox-cli run com.canonical.certification::networking/http
===========================[ Running Selected Jobs ]============================
=========[ Running job 1 / 1. Estimated time left (at least): 0:00:00 ]=========
-----------[ Ensure downloading files through HTTP works correctly. ]-----------
ID: com.canonical.certification::networking/http
Category: com.canonical.plainbox::networking
... 8< -------------------------------------------------------------------------
URL transformed to HTTPS due to an HSTS policy
--2024-09-04 16:53:57--  https://ubuntu.com/
Resolving ubuntu.com (ubuntu.com)... 185.125.190.29, 185.125.190.21, 185.125.190.20, ...
Connecting to ubuntu.com (ubuntu.com)|185.125.190.29|:443... connected.
HTTP request sent, awaiting response... 
  HTTP/1.1 200 
  server: nginx/1.14.0 (Ubuntu)
  date: Wed, 04 Sep 2024 14:53:58 GMT
  content-type: text/html; charset=utf-8
  content-length: 119652
  x-view-name: canonicalwebteam.templatefinder.templatefinder.template_finder
  x-clacks-overhead: GNU Terry Pratchett
  permissions-policy: interest-cohort=()
  cache-control: max-age=60, stale-while-revalidate=86400, stale-if-error=300
  x-content-type-options: NOSNIFF
  x-vcs-revision: 1725458684-6a26498
  x-request-id: ab5646487d3efa085b92baabd9097b69
  strict-transport-security: max-age=15724800
  link: <https://assets.ubuntu.com>; rel=preconnect; crossorigin, <https://assets.ubuntu.com>; rel=preconnect, <https://res.cloudinary.com>; rel=preconnect
  x-cache-status: HIT from content-cache-il3/2
  accept-ranges: bytes
Length: 119652 (117K) [text/html]
Saving to: ‘/dev/null’

     0K .......... .......... .......... .......... .......... 42%  391K 0s
    50K .......... .......... .......... .......... .......... 85% 1.17M 0s
   100K .......... ......                                     100% 2.06M=0.2s

2024-09-04 16:53:58 (658 KB/s) - ‘/dev/null’ saved [119652/119652]

Attempt 1/5
===========

------------------------------------------------------------------------- >8 ---
Outcome: job passed
Finalizing session that hasn't been submitted anywhere: checkbox-run-2024-09-04T14.53.51
==================================[ Results ]===================================
 ☑ : Ensure downloading files through HTTP works correctly.

And it tries 5 times with a moving delay when failing (here, the HTTP URL to use has been left empty, so wget fails with an Invalid host name error):

$ checkbox-cli run com.canonical.certification::networking/http
===========================[ Running Selected Jobs ]============================
=========[ Running job 1 / 1. Estimated time left (at least): 0:00:00 ]=========
-----------[ Ensure downloading files through HTTP works correctly. ]-----------
ID: com.canonical.certification::networking/http
Category: com.canonical.plainbox::networking
... 8< -------------------------------------------------------------------------
http://: Invalid host name.
http://: Invalid host name.
http://: Invalid host name.
http://: Invalid host name.
http://: Invalid host name.

===========
Attempt 1/5
===========
Attempt 1 failed:
Command '['wget', '-SO', '/dev/null', 'http://']' returned non-zero exit status 1.

Waiting 20.92 seconds before retrying...

===========
Attempt 2/5
===========
Attempt 2 failed:
Command '['wget', '-SO', '/dev/null', 'http://']' returned non-zero exit status 1.

Waiting 27.41 seconds before retrying...

===========
Attempt 3/5
===========
Attempt 3 failed:
Command '['wget', '-SO', '/dev/null', 'http://']' returned non-zero exit status 1.

Waiting 27.43 seconds before retrying...

===========
Attempt 4/5
===========
Attempt 4 failed:
Command '['wget', '-SO', '/dev/null', 'http://']' returned non-zero exit status 1.

Waiting 44.94 seconds before retrying...

===========
Attempt 5/5
===========
Attempt 5 failed:
Command '['wget', '-SO', '/dev/null', 'http://']' returned non-zero exit status 1.

All the attempts have failed!
Command '['wget', '-SO', '/dev/null', 'http://']' returned non-zero exit status 1.
------------------------------------------------------------------------- >8 ---
Outcome: job failed
Finalizing session that hasn't been submitted anywhere: checkbox-run-2024-09-11T07.50.18
==================================[ Results ]===================================
 ☒ : Ensure downloading files through HTTP works correctly.

@Retry

When a function is decorated with @Retry, it will be retried until it passes or until the maximum defined attempts have been reached: >>> from checkbox_support.helpers.retry import retry >>> result my_failing_test() KeyboardInterrupt >>> @Retry(max_attempts=2, max_delay=10) ... def my_failing_test(arg): ... return 1 / arg ... >>> result = my_failing_test(0) Attempt 1/2 =========== Attempt 1 failed: division by zero Waiting 2.22 seconds before retrying... Attempt 2/2 =========== Attempt 2 failed: division by zero All the attempts have failed! >>> @Retry(max_attempts=2, max_delay=10) ... def my_passing_test(arg): ... return 1 * arg ... >>> result = my_passing_test(0) Attempt 1/2 =========== >>> result 0

codecov · 2024-09-04T15:25:53Z

Codecov Report

Attention: Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.

Project coverage is 46.23%. Comparing base (0881101) to head (73481d7).
Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
providers/base/bin/networking_http.py	75.00%	2 Missing ⚠️
checkbox-support/checkbox_support/helpers/retry.py	97.67%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1453      +/-   ##
==========================================
+ Coverage   45.87%   46.23%   +0.36%     
==========================================
  Files         367      368       +1     
  Lines       39156    39243      +87     
  Branches     6622     6636      +14     
==========================================
+ Hits        17961    18144     +183     
+ Misses      20504    20401     -103     
- Partials      691      698       +7

Flag	Coverage Δ
checkbox-support	`60.29% <97.67%> (+0.77%)`	⬆️
provider-base	`20.59% <75.00%> (+0.67%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hook25

Nicely done! I like this
A few changes, major I would like to see is the patch, it makes testing this easier (mocking decorators is tricky). Also, please don't change the exception, I see no upside to that tbh.

checkbox-support/checkbox_support/helpers/retry.py

checkbox-support/checkbox_support/tests/test_retry.py

providers/base/tests/test_networking_http.py

checkbox-support/checkbox_support/helpers/retry.py

@Retry

- Raise the encountered exception (instead of SystemExit) when all the attempts have been tried - Separate the actual retry code into its own function, `run_with_retry()`: this allows the retry command to be used without a decorator - Remove initial delay and backoff factor from function signatures: these are not modified often in practice, whereas max_attempts and delay are more useful - Make the decorator arguments mandatory: instead of using default values, let the test author decide how many attempts and what delay to use when using the @Retry decorator - Add a `fake_run_with_retry()` function to facilitate unit testing of code using the retry decorator

If test author try to use attempts or delay with a value less than 1, ValueError is raised.

pieqq added 2 commits September 4, 2024 16:44

Modify the networking_http.py script to use the @Retry decorator

16a2728

Hook25 requested changes Sep 4, 2024

View reviewed changes

pieqq added 4 commits September 10, 2024 21:11

Modify networking HTTP script following retry decorator refactoring

7ae34dd

black formatting

2937c82

Handle negative attempts and delay

73481d7

If test author try to use attempts or delay with a value less than 1, ValueError is raised.

Hook25 approved these changes Sep 11, 2024

View reviewed changes

pieqq merged commit 99f7a0c into main Sep 11, 2024

pieqq deleted the 1543-jitter-backoff-decorator branch September 11, 2024 08:24

pieqq mentioned this pull request Sep 19, 2024

Use the retry decorator in WiFi tests (Bugfix) #1488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide a @retry decorator to automatically retry Python functions in Checkbox jobs (new)#1453

Provide a @retry decorator to automatically retry Python functions in Checkbox jobs (new)#1453
pieqq merged 6 commits intomainfrom
1543-jitter-backoff-decorator

pieqq commented Sep 4, 2024 •

edited

Loading

Uh oh!

codecov bot commented Sep 4, 2024 •

edited

Loading

Uh oh!

Hook25 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pieqq commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Resolved issues

Documentation

Tests

Uh oh!

codecov bot commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Hook25 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pieqq commented Sep 4, 2024 •

edited

Loading

codecov bot commented Sep 4, 2024 •

edited

Loading