[pawank925]

changes

Make save_large_file robust to interrupted downloads, as reported in #348.

save_large_file shelled out to a single wget -t {retries} ... -c run. The issue is not only that wget treats some transient errors as fatal: wget's -t is 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:

• the resume point is derived from the bytes already on disk and sent as a Range request, so an interrupted download continues instead of restarting;
• the retry counter 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 is never abandoned early;
• retries happen on connection/timeout/chunked-encoding errors and on the same transient HTTP statuses the existing get_retry_adapter retries, with exponential backoff;
• any other error (e.g. a 404) raises the new DownloadError immediately;
• if the server ignores the Range request (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.get mocked) 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 after retries consecutive no-progress attempts, and continuing past retries attempts as long as each makes progress. The existing slow tests exercise real downloads, and the ogvjs/zim suites that use save_large_file still pass.

Closes #348.

[pawank925]

@benoit74

[benoit74]

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.

[pawank925]

The issue is that wget's -t is a budget for the entire download, not consecutive failures. When a flaky connection keeps dropping mid-way, it exhausts that budget even though each attempt resumes correctly.
I rewrote save_large_file to stream with requests, resume via the Range header, and only count attempts that make no progress against the retry limit — so a download that keeps progressing keeps retrying. Transient HTTP statuses, connection/timeout/chunked-encoding errors get exponential backoff, other errors raise DownloadError, and we restart if the server ignores Range.
This mirrors zimwright's approach. The unit tests cover the key cases and the slow tests still pass. All checks are clean.

@benoit74 once check please thanks!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (cc3723c) to head (1353fa9).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #349   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         2729      2799   +70     
  Branches       406       417   +11     
=========================================
+ Hits          2729      2799   +70     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[benoit74]

@pawank925 please also do not merge from main to feature branch, this is against our practices and just adds noise (if you really need a change done in main, then you have to rebase and force push ; if review is OK, then you rebase and force push ; otherwise, you keep the branch a bit outdated, this eases the review process)

[benoit74]

@pawank925 please adopt latest changes in zimwright around this function (hence my initial recommendation to not start working on this too fast) and ensure code coverage is 100%

[pawank925]

Rebased without merging and ported the latest zimwright save_large_file changes. Patch coverage is 100%.

[benoit74]

LGTM, thank you!