-
-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check MD5 sums of downloaded atom data #2267
Conversation
@@ -396,9 +397,24 @@ def download_from_url(url, dst, src=None): | |||
URL to download from | |||
dst : str | |||
Destination folder for the downloaded file | |||
src : list | |||
src : tuple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the parameters must be hashable to memoize with lru_cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is definitely the best way to deal with the problem
*beep* *bop* Hi, human. The Click here to see your results. |
…oader functions to check-checksums; Add retrying capabilities; Logging
d7d8289
to
cf43c21
Compare
Rebased |
Codecov Report
@@ Coverage Diff @@
## master #2267 +/- ##
==========================================
- Coverage 71.88% 71.83% -0.05%
==========================================
Files 133 133
Lines 12384 12396 +12
==========================================
+ Hits 8902 8905 +3
- Misses 3482 3491 +9
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this pull request. Sorry, it took me a while to review this.
* Add MD5 as a value on atom data YAML configuration file; Modify downloader functions to check-checksums; Add retrying capabilities; Logging * Sort modules * Reformatted with black
📝 Description
Type: 🪲
bugfix
| 🎢infrastructure
This is an attempt to fix the random failure of the documentation pipeline. I always had the suspicion that HDF5 files sometimes are corrupted, but I can't be sure (
libhdf5
is an awful library).In this PR I made the changes necessary to compare the known MD5 checksums of atomic files with the downloaded ones through the
download_atom_data
function. Also, allows to set a maximum number of retries.Fixes #2252
📌 Resources
🚦 Testing
How did you test these changes?
Locally, modify a digit in the
md5
key of thetardis/data/atomic_data_repo.yml
file and call the function, see how in behaves until reaches the limit.☑️ Checklist
build_docs
label