Skip to content
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

Harden download robustness #5545

Merged
merged 21 commits into from Nov 27, 2017
Merged

Conversation

benjaoming
Copy link
Contributor

@benjaoming benjaoming commented Nov 20, 2017

Summary

  • Fixes blank video downloads when server is down (part of Video download robustness improvements #5538)
  • Raises exceptions when non-200 download responses
  • Resumes video / content pack download in case of connection disruptions during download (not in case the download is stopped/started)
  • Adds tests for various failures
  • Adds tests for downloading + deleting content packs
  • Deletes content pack database when deleting a content pack.
  • Fixed missing update from database when a video download gets canceled while being processed
  • Improves logging on many fronts
  • Feedback when downloading content packs retrievecontentpack download gives no feedback #5356

TODO

If not all TODOs are marked, this PR is considered WIP (work in progress)

  • Has documentation been written/updated?
  • Have you written release notes for the upcoming release?
  • Allow to fail gracefully if thumbnails are absent

Reviewer guidance

...

Issues addressed

#5538
#5356

…sponses, add tests for various failures, improve logging
@codecov
Copy link

codecov bot commented Nov 20, 2017

Codecov Report

Merging #5545 into 0.17.x will increase coverage by 1.89%.
The diff coverage is 73.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           0.17.x    #5545      +/-   ##
==========================================
+ Coverage   64.67%   66.56%   +1.89%     
==========================================
  Files         117      117              
  Lines        6612     6639      +27     
==========================================
+ Hits         4276     4419     +143     
+ Misses       2336     2220     -116
Impacted Files Coverage Δ
kalite/updates/settings.py 100% <ø> (ø) ⬆️
kalite/topic_tools/content_models.py 77.8% <0%> (-0.29%) ⬇️
kalite/updates/models.py 84.82% <100%> (+0.13%) ⬆️
kalite/updates/management/utils.py 78.44% <100%> (+5.88%) ⬆️
...alite/updates/management/commands/videodownload.py 74.56% <62.5%> (+15.73%) ⬆️
kalite/updates/videos.py 88.73% <73.33%> (+2.79%) ⬆️
...ributed/management/commands/retrievecontentpack.py 89.16% <76.92%> (+40.98%) ⬆️
kalite/i18n/base.py 76.17% <81.25%> (+20.31%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86e1ea6...e941add. Read the comment docs.

@benjaoming
Copy link
Contributor Author

Added download progress for contentpack retrieval.

image

@benjaoming
Copy link
Contributor Author

Download progress callback also added in UI

image

@benjaoming
Copy link
Contributor Author

Repositioned the progress bars for content pack downloads so it doesn't look like something is broken.

image

@benjaoming
Copy link
Contributor Author

A small related TODO of this PR would be to add automated testing for retrieving and deleting a content pack.

@benjaoming
Copy link
Contributor Author

Merging this for further tests in a pre-release!

@benjaoming benjaoming merged commit 25c954e into learningequality:0.17.x Nov 27, 2017
@benjaoming benjaoming deleted the bug/video_url branch December 7, 2017 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant