Resume incomplete download

[yichi-yang]

Overview

This PR adds a feature that resumes download if the downloaded file is incomplete (e.g. when the Internet connection is poor). More specifically, if :

1. The initial response includes a Content-Length header,
2. The server sends fewer bytes than indicated in the Content-Length header,
3. The user enables the auto resumption feature using --incomplete-downloads=resume,

the downloader will make new requests and attempt to resume download using a Range header. If the initial response includes an ETag (preferred) or Date header, the downloader will ask the server to resume download only when it is safe (i.e., the file hasn't changed since the initial request) using an If-Range header.

If the server responds with a 200 (e.g. if the server doesn't support partial content or can't check if the file has changed), the downloader will restart the download (i.e. start from the very first byte); if the server responds with a 206 Partial Content, the downloader will resume the download from the partially downloaded file.

Note if the server always responds with 200, the downloader can potentially get stuck and waste unreasonable amounts of bandwidth downloading the first few bytes over and over again. Therefore, a retry limit is introduced to avoid this case.

If not enough bytes are received and auto resumption is disabled or the retry limit is exceeded, the downloader will clean up the incomplete file and fail with an exception.

Flags

To control the auto resumption behavior, two new flags are added:

• --incomplete-downloads=resume,/discard controls whether the auto resumption feature is enabled (defaults to discard);
• --incomplete-download-retries limits the maximum number of retries (defaults to 5).

---

Towards #4796

[yichi-yang]

Not quite sure what to do here. I don't think throwing an exception (L244) and printing a long (and useless) stack trace is user-friendly, but I can't think of a better alternative. Maybe we should just reuse the the same log messages above so it's at least helpful?
I think throwing an (subclassed) DiagnosticPipError here might be a good idea? We can let the user know that:

1. the download is incomplete
2. the incomplete file has been cleaned up
3. they can use --incomplete-downloads=resume to enable the feature if they haven't already
4. they can modify the retry limit with --incomplete-download-retries.

pip/src/pip/_internal/exceptions.py

Lines 54 to 63 in e5898ab

	class DiagnosticPipError(PipError):
	"""An error, that presents diagnostic information to the user.
	This contains a bunch of logic, to enable pretty presentation of our error
	messages. Each error gets a unique reference. Each error can also include
	additional context, a hint and/or a note -- which are presented with the
	main error message in a consistent style.
	This is adapted from the error output styling in `sphinx-theme-builder`.
	"""

[yichi-yang]

Implemented the above in a separate commit: a082517.

[uranusjr]

I saw that a previous version also queries Accept-Ranges to determine whether to go into using Range for resuming. Was it removed because it’s considered not a good idea?

[uranusjr]

I wonder (discussions needed!) if we should aggressively set the default to resume. The logic seems safe enough and without much overhead (and the worse case scenario can only be caused by incorrect server implementation) that we should make the benefit opt-out instead opt-in.

[yichi-yang]

I saw that a previous version also queries Accept-Ranges to determine whether to go into using Range for resuming. Was it removed because it’s considered not a good idea?

As per RFC 7233 servers that don't support Range will simply ignore it and respond with 200 as if it is a normal GET request. Additionally, if the server supports Range but the document has changed since the initial request, we need to somehow figure that out and restart instead of resume (that's what the If-Range header does).

So to answer your question, checking Accept-Ranges is kind of redundant here (it's up to the server to decide if a partial response is appropriate). I don't think we should check Accept-Ranges unless there's a very good reason to do so.

[yichi-yang]

I think it might make more sense if we only have a single --incomplete-download-retries flag - setting it to 0 or a negative number disables the feature. This is more similar to how --retries works currently.

[uranusjr]

Ah yes that makes sense (perhaps with a shorter option name though)

[yichi-yang]

(perhaps with a shorter option name though)

Suggestions are welcome :)
Though we need to make sure users won't confuse this with --retries.

[CTimmerman]

Just use --retries. It's the same as this with 0 bytes.

[yichi-yang]

Just use --retries. It's the same as this with 0 bytes.

Could you elaborate? I don't think --retries on its own solves the issue #4796 here.

[uranusjr]

I think it’s “just use --retries to control resuming”, in the sense that right now a retry is conceptually the same as resuming from a previous download of zero bytes.

[yichi-yang]

I think it’s “just use --retries to control resuming”, in the sense that right now a retry is conceptually the same as resuming from a previous download of zero bytes.

Ah I see. Our resume function is conceptually similar to --retries, but implementation-wise --retries are handled by requests/urllib3. I don't really know a good way to combine the two. Consider the case where a user sets --retries 5, does it mean requests can make 5 retries AND pip can try resuming 5 times (kind of weird), or requests and pip can retry a total of 5 times (hard to implement, retry now means two different things)?

On a side note, part of the problem should get fixed upstream soon (psf/requests#4956, psf/requests#6092), but that doesn't change the fact that pip has to make additional partial requests to resume.

[uranusjr]

Yeah that’s the problem, we can’t really use the same counter between connection retries and resumes. But I’m guessing it’s not that big a problem and we could intentionally implement the wrong behaviour and find a way to fix that later… For example for retries=5 we would do 5 connection retries and 5 resume retries, and most of the time the user wouldn’t tell the difference, since it’s relatively rare to see a connection fails halfway, and when resuming starting to fail to connect. And even in that case users might be fine to do a few more retries, who knows. I think I’d prefer to get the user interface (CLI flags) right, and “fix” the relatively minor behaviour later (if ever).

[yichi-yang]

Yeah that’s the problem, we can’t really use the same counter between connection retries and resumes. But I’m guessing it’s not that big a problem and we could intentionally implement the wrong behaviour and find a way to fix that later… For example for retries=5 we would do 5 connection retries and 5 resume retries, and most of the time the user wouldn’t tell the difference, since it’s relatively rare to see a connection fails halfway, and when resuming starting to fail to connect. And even in that case users might be fine to do a few more retries, who knows. I think I’d prefer to get the user interface (CLI flags) right, and “fix” the relatively minor behaviour later (if ever).

Good point. But that's an easy change so I think we can wait a bit and see what others think.

[CTimmerman]

I think the default 5 connection retries should include resuming, and that resuming should not start from 0 by default, but would not mind reusing the user's --retries value if the actual one is not available. Resuming should be an upstream feature if retrying is.

[horacehoff]

Imo the download shouldn't resume by starting by 0. Let it resume using the already-partially-downloaded file.

[yichi-yang]

Imo the download shouldn't resume by starting by 0. Let it resume using the already-partially-downloaded file.

The implementation in this PR resumes from partially downloaded file when possible. There are cases where resuming is not possible, e.g. when the file has changed on the server after we started the download, and we have to start downloading from scratch again.

Is that the behavior you want?

[horacehoff]

Oh right, sorry. I misunderstood exactly what this PR does and from this new point of view I have nothing to say as it sounds much useful to many people who have poor/low bandwidth.

[JD91B]

I was having problems no being able to download large files with pip because of my slow internet but this helped so much. Thank you, it was really useful.