Fix bugs in resumable_download
#1135
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csukuangfj
reviewed
Sep 6, 2023
| @@ -513,7 +513,7 @@ def _download(rq, size): | |||
| # sometimes, the server actually supports range requests | |||
| # but does not return the Content-Range header with 416 code | |||
| # This is out of spec, but let us check twice for pragmatic reasons. | |||
| head_req = urllib.request.Request(url, method='HEAD') | |||
| head_req = urllib.request.Request(url, method="HEAD") | |||
There was a problem hiding this comment.
I believe "HEAD" and 'HEAD' are equivalent in Python. That is
assert "HEAD" == 'HEAD' == """HEAD""" == '''HEAD'''There was a problem hiding this comment.
@csukuangfj yes, the name of the commit is "run pre-commit" (to make CI happy), this is the edit by black formatter
pzelasko
approved these changes
Sep 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug #1: re-downloading from scratch worked incorrectly, since the file was opened in "ab" mode - hence, seeking to byte 0 had no effect, and the entire file was appended to the already downloaded part instead of rewriting it. Fixed this by opening the file in "r+b" mode (not "wb" to preserve the original contents).
Bug #2 (which is not really a bug but still): some servers serving some datasets behave incorrectly while returning 416 status code - they do not supply the "Content-Range" header with the response, even though they accept ranged downloads, as indicated by "Accept-Ranges" header in 200/206 responses. The example of that is AMI dataset server.
These bugs were encountered by me when trying to resume an interrupted download of AMI dataset, which resulted in a huge mess in the corpus folder. With those fixes, interrupting and resuming the download works without problems.