Repository navigation
Conversation
816a90d to
1b7a481
Compare
|
Thank you, but it looks like this solution does not allowed zimwright to complete the download, so I'm not yet convinced this is the proper one to implement. |
1b7a481 to
744fd1e
Compare
save_large_file() shelled out to a single wget run with -t {retries}. wget's
-t is a budget for the whole download, not for consecutive failures: a
connection that drops every few hundred MB exhausts it and aborts with no
further retry, even though every attempt resumes correctly. That is what made
long downloads fail in production (see openzim/zimwright#3).
Stream the response with requests instead, deriving the resume point from the
bytes already on disk and sending a Range request. The retry counter now only
grows on attempts that make no progress at all: any attempt that writes at
least one byte resets it, so a flaky-but-advancing download keeps resuming
until it completes. Transient HTTP statuses (matching the existing requests
retry adapter) and connection/timeout/chunked-encoding errors are retried with
exponential backoff; anything else raises the new DownloadError. If the server
ignores the Range request, the file is started over.
This is the same approach already used by zimwright, which can now rely on the
library implementation.
Closes openzim#348.
744fd1e to
7ba29ee
Compare
|
The issue is that wget's @benoit74 once check please thanks! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #349 +/- ##
===========================================
- Coverage 100.00% 99.92% -0.08%
===========================================
Files 42 42
Lines 2729 2765 +36
Branches 406 412 +6
===========================================
+ Hits 2729 2763 +34
- Misses 0 1 +1
- Partials 0 1 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
changes
Make
save_large_filerobust to interrupted downloads, as reported in #348.save_large_fileshelled out to a singlewget -t {retries} ... -crun. The issue is not only that wget treats some transient errors as fatal: wget's-tis a budget for the whole download, not for consecutive failures. A connection that drops every few hundred MB therefore exhausts the budget and aborts with no further retry, even though every attempt resumes correctly — which is what was observed in openzim/zimwright#3.fix
Replace the wget shell-out with a streaming download built on
requests:Rangerequest, so an interrupted download continues instead of restarting;get_retry_adapterretries, with exponential backoff;404) raises the newDownloadErrorimmediately;Rangerequest (replies 200 to a range request), the file is started over.This matches the approach already used by zimwright, which can now rely on the library implementation instead of keeping its own copy.
Tests
New unit tests (no network,
requests.Session.getmocked) cover a clean download, resume after a dropped connection, retry on a transient HTTP status, restart when the server ignores the range, no retry on a non-retryable status, giving up afterretriesconsecutive no-progress attempts, and continuing pastretriesattempts as long as each makes progress. The existing slow tests exercise real downloads, and theogvjs/zimsuites that usesave_large_filestill pass.Closes #348.