Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Skip ETag check if OSError #4469

Merged
merged 4 commits into from
Jul 13, 2020
Merged

Skip ETag check if OSError #4469

merged 4 commits into from
Jul 13, 2020

Conversation

JohnGiorgi
Copy link
Contributor

@JohnGiorgi JohnGiorgi commented Jul 13, 2020

Based on the discussion here, attempts to fix an issue that was preventing cached_path from being used to download files from GitHub releases.

It looks like ETags are optional, so I elected to just catch the OSError that is thrown by _http_etag and set etag = None in this case (url_to_filename already supports an unspecified etag). This solution seems cleaner to me than asking the user to provide some flag to skip the ETag fetch.

Let me know if you are happy with this solution and then I can try to write a unit test for this scenario / update the CHANGELOG.md.

Copy link
Member

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Can you just add a note to the CHANGELOG under the "Fixed" section?

@JohnGiorgi
Copy link
Contributor Author

Done :)

CHANGELOG.md Outdated Show resolved Hide resolved
@epwalsh
Copy link
Member

epwalsh commented Jul 13, 2020

Thank you very much!

@epwalsh epwalsh merged commit 64db027 into allenai:master Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants