Remove video_shorts pipeline#2253
Open
mbertrand wants to merge 3 commits into
Open
Conversation
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the legacy video_shorts Dagster pipeline from the learning_resources code location, including its assets/sensors/helpers and related local configuration, while keeping the replacement ovs_videos pipeline intact.
Changes:
- Deletes
video_shortsassets, sensors, and helper modules (Google Sheets + ffmpeg utilities). - Removes the Learn API client hook for
video_shortswebhooks. - Drops
VIDEO_SHORTS_*env var wiring from local dev configuration and removesvideo_shortsregistrations fromlearning_resources/definitions.py.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ol-orchestrate-lib/src/ol_orchestrate/resources/learn_api.py | Removes the notify_video_shorts webhook client method. |
| docker-compose.yaml | Removes VIDEO_SHORTS_* environment variables from the learning_resources code location container. |
| dg_projects/learning_resources/learning_resources/sensors/video_shorts.py | Deletes all Video Shorts sensors/jobs definitions. |
| dg_projects/learning_resources/learning_resources/lib/video_processing.py | Deletes ffmpeg-based thumbnail/compression helpers used by Video Shorts. |
| dg_projects/learning_resources/learning_resources/lib/google_sheets.py | Deletes Google Sheets parsing/fetching helpers used by Video Shorts. |
| dg_projects/learning_resources/learning_resources/lib/contants.py | Deletes Video Shorts thumbnail dimension constants module. |
| dg_projects/learning_resources/learning_resources/definitions.py | Removes Video Shorts resources/assets/schedules/sensors/jobs from Dagster Definitions (but currently introduces a duplication issue). |
| dg_projects/learning_resources/learning_resources/assets/video_shorts.py | Deletes all Video Shorts assets and partitions definitions. |
| .env.example | Removes Video Shorts-related example env vars. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on PR #2253: - Drop duplicated `ovs_videos` sensor import block - Drop second copy of `ovs_videos_api_job`, `ovs_videos_webhook_job`, and `ovs_videos_api_schedule` - Remove unused `MIT_LEARN_BUCKET_SUFFIXES` constant Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Marking this as blocked until the mit-learn video shorts carousel has been displaying learning resources for a few days, to make sure everything works as expected.
What are the relevant tickets?
Closes mitodl/hq#11470
Description (What does it do?)
Removes the
video_shortspipeline from thelearning_resourcesDagster code location.The replacement
ovs_videospipeline (assets + sensors) remains in place and is unaffected.How can this be tested?
If you already have video_shorts materializations:
On this branch:
docker compose up --buildand wait for thelearning_resourcescode location to come up.video_shortsartifacts exist:video_shorts— should be empty)video_shorts_*sensor)video_shorts_*job)ovs_videospipeline is still present:ovs_videos_discovery_sensor,ovs_videos_stale_cleanup_sensor,ovs_videos_delete_partition_cleanup_sensorlisted at http://localhost:3000/locations/learning_resources/sensorsovs_videos_delete_joblisted at http://localhost:3000/locations/learning_resources/jobs