Implement support for downloading videos from Google Drive - #4715
Merged
Conversation
Rename WetransferToS3TransferRequest to VideosImportRequest and its wetransfer_url field to source_url, ahead of adding Google Drive as a second import provider. Provider-neutral names let one model, one status machine and one admin page serve both WeTransfer and Drive imports. Migration 0002 is a pure RenameModel + RenameField (verified via sqlmigrate to emit ALTER TABLE ... RENAME, no data mutation). is_s3_storage switches from == to is: pre-existing E721 that pre-commit only surfaces now that the file is touched. Comparing type objects with is has identical semantics, so the exact-type check is unchanged. Celery task, admin helper and log strings keep their old names for now; they are renamed in the following commit to keep this diff mechanical.
…request Complete the provider-neutral rename: the Celery task, the admin queueing helper, the processing class attribute, log messages and temp file prefixes all drop the wetransfer name. Only get_download_link keeps WeTransfer-specific local names, since that method is WeTransfer's link resolution and stays provider-specific. OnlyOneAtTimeTask derives its Redis lock key from the task module and name, so the rename changes the lock key. Requests left QUEUED across the deploy are re-triggered from the admin retry action.
…factory Split transfer.py into the provider-agnostic blob pipeline and the WeTransfer provider on top of it, so Google Drive can reuse the ranged multi-part download, zip extraction and S3 upload machinery. BaseTransferProcessing.import_blob imports whatever the subclass points download_link/filename/extension/transfer_total_size at, which lets a provider call it once per file when importing a folder. Providers add auth via the download_headers hook, merged with the Range header on every part attempt so a token refreshed mid-download is picked up. WeTransfer returns no extra headers, so its requests are unchanged - asserted by a new test over the full request history. get_processing_class maps hostname to provider and raises UnsupportedVideoImportUrlError otherwise. This replaces the implicit "any hostname works" behaviour of the old code, which built the WeTransfer API URL from whatever host the user pasted.
…aved Reject unsupported import URLs when an organizer saves the form, instead of accepting the request and failing later inside the Celery task. Validation delegates to get_processing_class, so Drive URLs become valid automatically once that provider is registered - the admin needs no further change.
The quota machinery resolves the limit column by interpolating the
service name, so adding quota_limit_for_drive is all that is needed for
with_quota_left("drive") and get_available_credentials_token("drive", n)
to work.
Drive's real limits are per-user rates rather than a daily bucket, so
this is a safety valve against runaway imports, not an enforcement of
Google's own limits.
Add the drive.readonly scope and three helpers the video import pipeline needs: refreshed credentials for callers that issue their own ranged media downloads, single file metadata, and a paginated folder listing. The helpers call the Drive REST API with requests rather than googleapiclient so that tests can intercept them, and so the same Bearer token can be reused for the ranged download in video_uploads. Credentials rebuilt from a stored token are always expired, because from_authorized_user_info records no expiry: googleapiclient hid this by refreshing lazily inside build(), but a raw requests call would send the stale token, so every helper refreshes explicitly first. Existing tokens only carry the YouTube scope. They must be re-authorized from the admin before the first Drive import, otherwise Drive answers 403.
Recognise the URL shapes organizers actually paste - /file/d/<id>/view, /file/d/<id>/edit, /open?id=<id>, /drive/folders/<id> and the /drive/u/<n>/folders/<id> variant produced when several Google accounts are signed in - and reject anything else with a message naming the two supported shapes.
Add the Drive provider for file links: read name, size and mimeType from the Drive API, then pull the bytes through the existing blob pipeline via the alt=media endpoint, which honours Range requests so large recordings still download as parallel parts. A zip on Drive is unzipped exactly like a WeTransfer one. download_headers is re-evaluated on every part attempt under a lock, so a token that expires part-way through a long download is refreshed instead of failing the import. Google-native files carry no downloadable bytes, so a file link pointing at a Doc, Sheet or Slides fails with a message telling the organizer to export and share a regular file.
Walk a shared folder depth-first, following nextPageToken, and import every file through the same blob pipeline, storing each one under its path relative to the shared folder so the videographer's structure survives the import. Docs, Sheets, Slides and shortcuts have no bytes behind them. They are logged and skipped rather than failing the folder, which matches how zip imports already skip junk entries.
Translate the three failures an organizer can actually act on into messages that say what to do: no Drive-capable credential, Google refusing access, and a link that points at nothing reachable. NoGoogleCloudQuotaLeftError carries no message at all, so without this it reached the admin as an empty failed_reason.
Drive the whole path through the Celery task with only HTTP mocked, so the task, the provider factory, the Drive provider and the storage write are exercised together rather than behind a patched run(). Also check the admin accepts both Drive link shapes and queues them onto heavy_processing, the same queue WeTransfer imports use.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| with pytest.raises(UnsupportedVideoImportUrlError) as exc: | ||
| get_processing_class("https://example.com/some-video.mp4") | ||
|
|
||
| assert "example.com" in str(exc.value) |
Say "WeTransfer or Google Drive" when rejecting an unsupported URL: the message still named only WeTransfer, from before the Drive provider existed. Drop the unused imported_files attribute on the processing object. Both providers return their list directly and the model field of the same name is what actually gets written. Move the Drive mime helpers above their only caller and derive the folder type from the shared google-apps prefix, which makes it visible that folders match is_google_native too and so must be ruled out first. Rename import_drive_file to import_file, since inside GoogleDriveProcessing every method imports a Drive file. Its caller becomes import_file_by_id, naming how the file is addressed rather than repeating the provider. Build the folder listing params once instead of rebuilding them per page.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4715 +/- ##
==========================================
+ Coverage 92.43% 92.50% +0.07%
==========================================
Files 355 355
Lines 10719 10877 +158
Branches 818 837 +19
==========================================
+ Hits 9908 10062 +154
- Misses 698 699 +1
- Partials 113 116 +3 🚀 New features to boost your workflow:
|
Contributor
|
Adds Google Drive as a second source for video imports, alongside a refactor that generalizes the wetransfer-only transfer pipeline (WetransferProcessing into BaseTransferProcessing/GoogleDriveProcessing, model rename to VideosImportRequest) and per-credential Drive quota tracking. A few issues in the Drive quota/error-handling paths:
|
The old assertion only checked that the refused hostname was echoed back, through str() of an ErrorList, which renders HTML. That let two real regressions through: the message naming only WeTransfer months after Drive shipped, and the message stripped to a bare hostname. Both were verified to keep the old test green. Assert on the raw error instead: exactly one error, on source_url alone. Then guard the two things the message is actually for. The staleness bug cannot be caught by asserting on copy, because the copy never changed - the provider registry did. So the second test compares PROCESSING_CLASSES_BY_HOSTNAME against the test's own table of names an organizer must see, and fails if a provider is registered without anyone revisiting the wording. It keys on the provider class rather than the hostname, so adding a host alias costs nothing but adding a provider stops the suite. The lookalike cases pin the allowlist as the SSRF control it replaced: exact-match hostnames, so wetransfer.com.evil.example is refused, and a userinfo host like drive.google.com@169.254.169.254 is reported as the metadata address it really resolves to. Verified by mutation: stale copy, stripped copy, dropped hostname, a lookalike added to the allowlist, a suffix-matching allowlist, a third provider left out of the message, and validation removed altogether are each caught. Production code is unchanged.
marcoacierno
changed the title
GoogleDriveProcessing resolved credentials once and used them for the byte downloads, but drive_file_metadata and drive_list_files_in_folder took none, so count_quota resolved a fresh account for each of them. With more than one Drive-capable credential the metadata lookup, the folder listing and the download could run as different Google accounts, and a folder shared with only one of them failed with a 403 on files the organizer could plainly see. count_quota now accepts credentials the caller already holds and reuses them instead of picking an account, still charging the quota. Passing them was previously impossible: it collided with the injected keyword and raised TypeError. Callers that pass nothing are unaffected, so the YouTube helpers keep resolving per call as before. The regression test gives the first account exactly enough quota for one call, so a second lookup would fall through to the other account, and asserts every Drive request in the import carries the same bearer token. It fails if either call stops being handed the pinned credentials.
marcoacierno
marked this pull request as ready for review
August 7, 2026 20:53
Unzipping saved every entry under its zip-internal name alone, so a zip
imported from a Drive subfolder lost that subfolder: day2/videos.zip
holding recording.mp4 landed at conference-videos/{code}/recording.mp4
rather than under day2/. Plain files kept their relative path, so
imported_files mixed prefixed and unprefixed entries, and two subfolders
each shipping a zip with the same entry name silently overwrote one
another in storage.
Treat a zip as a container whose entries belong where the zip sits, and
report the same path in imported_files. A zip at the import root has no
prefix, so WeTransfer imports are unchanged.
The regression test imports a folder with day1/ and day2/ each holding a
videos.zip containing recording.mp4, and checks both survive with their
own contents.
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
2 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.
What
ToDo