fix: bound resource use in Studio video-download endpoint - #860
Merged
Conversation
The PUT /api/contentstore/v1/videos/{course_id}/download endpoint
returns a zip of selected course videos -- the operation is a
download. (The original implementation used PUT, which is the wrong
verb for a read; that's tracked as a separate cleanup.) The handler
synchronously fetched each requested video, materialised each
response in RAM and wrote it to a temp file before zipping. A course
author requesting download of many large videos in one request could
tie up a uWSGI worker for many minutes; concurrent requests exhausted
the worker pool.
Stream the zip directly into the response instead -- each video is
fetched with stream=True and piped chunk-by-chunk into a streaming zip
entry, so peak memory is roughly the chunk size and no temp files are
written to disk. Rate-limit the endpoint per user via the new
VIDEO_DOWNLOAD_RATE_LIMIT setting (default 12/hour, via a
UserRateThrottle subclass on the view). Per-request wall-clock is
bounded by the WSGI server's request timeout.
Closes GHSA-g266-6v7f-j465.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
@Kelketek the cherry pick looks good to me. Please take a look at the failing tests though. Thanks! :) |
Member
Author
|
@samuelallan72 I created an additional PR with an empty commit, and it failed with identical messages. Given that there are no new errors, I'm going ahead with merging. Looks like almost everyone will be off Teak soon anyway. |
Member
|
@Kelketek sounds good, thanks for checking 👍 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Per upstream:
The PUT /api/contentstore/v1/videos/{course_id}/download endpoint returns a zip of selected course videos -- the operation is a download. (The original implementation used PUT, which is the wrong verb for a read; that's tracked as a separate cleanup.) The handler synchronously fetched each requested video, materialised each response in RAM and wrote it to a temp file before zipping. A course author requesting download of many large videos in one request could tie up a uWSGI worker for many minutes; concurrent requests exhausted the worker pool.
Stream the zip directly into the response instead -- each video is fetched with stream=True and piped chunk-by-chunk into a streaming zip entry, so peak memory is roughly the chunk size and no temp files are written to disk. Rate-limit the endpoint per user via the new VIDEO_DOWNLOAD_RATE_LIMIT setting (default 12/hour, via a UserRateThrottle subclass on the view). Per-request wall-clock is bounded by the WSGI server's request timeout.
Closes GHSA-g266-6v7f-j465.
Internal ticket: https://tasks.opencraft.com/browse/BB-11055