-
Notifications
You must be signed in to change notification settings - Fork 20
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.
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
apt_dpkg: retry on HTTP 429 'Too Many Requests' #319
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #319 +/- ##
==========================================
+ Coverage 83.00% 83.02% +0.02%
==========================================
Files 99 99
Lines 20333 20357 +24
Branches 3212 3216 +4
==========================================
+ Hits 16877 16901 +24
+ Misses 2936 2935 -1
- Partials 520 521 +1 ☔ View full report in Codecov by Sentry. |
The retry does not seem to work as intended: https://github.com/bdrung/apport/actions/runs/8718363097/job/23915454500 |
We also have failing apt updates:
See https://github.com/bdrung/apport/actions/runs/8718363097/job/23915453469 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One small nitpick.
apt_cache.fetch_archives(fetcher=fetcher) | ||
break | ||
except apt.cache.FetchFailedException as error: | ||
if backoff <= 3600 and "Too Many Requests" in str(error): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: this is misleading wrt the git log
While it is technically correct because the git log says "around 1h" so if the next delay is 4096s we've already waited 4095s (yay maths), backoff
is still a derivation of the wall time and not the actual wall time. I'd really like a comment here to that effect like elapsed time is backoff-1 due to exponentiation
so that the next person reading this code doesn't have to come to that conclusion on their own.
The failing retries:
So something clearly does not work correctly when retrying. It should succeed after some time. |
Signed-off-by: Benjamin Drung <[email protected]>
Running the system tests on different releases in parallel can cause some tests of `test_packaging_apt_dpkg` fail: ``` __________________ test_install_packages_dependencies[deb822] __________________ [...] apport/packaging_impl/apt_dpkg.py:1326: SystemExit ----------------------------- Captured stderr call ----------------------------- ERROR: Package download error, try again later: Failed to fetch http://ddebs.ubuntu.com/pool/main/p/pcre2/libpcre2-8-0-dbgsym_10.39-3build1_amd64.ddeb 429 Too Many Requests [IP: 185.125.190.18 80] Failed to fetch http://ddebs.ubuntu.com/pool/main/libs/libselinux/libselinux1-dbgsym_3.3-1build2_amd64.ddeb 429 Too Many Requests [IP: 185.125.190.18 80] ``` Retry package download if HTTP 429 'Too Many Requests' errors can be found in the `apt.cache.FetchFailedException` error. The download is retries with an exponential backoff and giving up after around one hour. Signed-off-by: Benjamin Drung <[email protected]>
Running the system tests on different releases in parallel can cause some tests of
test_packaging_apt_dpkg
fail:Retry package download if HTTP 429 'Too Many Requests' errors can be found in the
apt.cache.FetchFailedException
error. The download is retries with an exponential backoff and giving up after around one hour.