Conversation
OpenAPI ChangesShow/hide 63 changes: 0 error, 1 warning, 62 infoUnexpected changes? Ensure your branch is up-to-date with |
f1b6925 to
97a325f
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for ingesting ODL Video Service (OVS) videos/collections into the learning resources system, including scheduled ETL runs and transcript backpopulation, and exposes the new platform + video fields via the API/OpenAPI.
Changes:
- Introduces an OVS ETL (extract/transform/load) that groups videos into playlists (collections) and loads them into
LearningResource/Video/VideoPlaylist. - Adds transcript-fetching job for OVS videos based on caption URLs, plus Celery beat schedules and a management command to backpopulate/delete.
- Extends the data model + OpenAPI + generated frontend API types to include
ovsplatform andVideo.caption_urls/Video.cover_image_url.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test_json/ovs_videos.json | Adds a fixture-style OVS API response used by OVS ETL tests. |
| openapi/specs/v1.yaml | Adds ovs to platform enums and adds caption_urls/cover_image_url to Video schemas. |
| openapi/specs/v0.yaml | Same as v1 for the v0 spec. |
| main/settings_course_etl.py | Adds OVS_API_BASE_URL setting used by the OVS extractor. |
| main/settings_celery.py | Schedules periodic OVS video ingestion and transcript update tasks. |
| learning_resources/tasks_test.py | Adds task-level tests for invoking the new OVS ETL/transcript functions. |
| learning_resources/tasks.py | Adds Celery tasks get_ovs_data and get_ovs_transcripts. |
| learning_resources/models.py | Adds caption_urls and cover_image_url fields to the Video detail model. |
| learning_resources/migrations/0106_video_caption_url_video_cover_image_url.py | Adds DB fields for Video.caption_urls and Video.cover_image_url. |
| learning_resources/migrations/0106_resource_category_choices_index.py | Removes a prior 0106 migration (replaced in this PR). |
| learning_resources/management/commands/backpopulate_ovs_data.py | Adds a management command to fetch/delete OVS data and (optionally) transcripts. |
| learning_resources/etl/pipelines.py | Wires a new ovs_etl pipeline into the ETL pipeline registry. |
| learning_resources/etl/ovs_test.py | Adds comprehensive unit tests for OVS extract/transform and transcript fetching. |
| learning_resources/etl/ovs.py | Implements OVS extraction (pagination), transform (collection grouping), and transcript fetching. |
| learning_resources/etl/loaders_test.py | Adds loader tests for OVS playlist loading/unpublishing behavior; updates playlist unpublish assertions. |
| learning_resources/etl/loaders.py | Adds load_ovs_playlist/load_ovs_playlists; adjusts playlist image handling and bulk-unpublish IDs. |
| learning_resources/etl/constants.py | Adds ovs to ETLSource enum. |
| learning_resources/constants.py | Adds ovs to PlatformType enum. |
| frontends/api/src/generated/v1/api.ts | Regenerates TS client types/enums to include ovs + the new video fields. |
| frontends/api/src/generated/v0/api.ts | Same as v1 for the v0 generated client. |
| data_fixtures/migrations/0024_add_ovs_platform_offeror.py | Adds a data migration to create the ovs platform record. |
| data_fixtures/fixtures/platforms.json | Adds ovs platform entry to fixture data. |
You can also share your feedback on Copilot code review. Take the survey.
learning_resources/migrations/0107_video_caption_url_video_cover_image_url.py
Outdated
Show resolved
Hide resolved
learning_resources/migrations/0107_video_caption_url_video_cover_image_url.py
Show resolved
Hide resolved
shanbady
left a comment
There was a problem hiding this comment.
looks good. just left a question
|
|
||
|
|
||
| class NullableURLField(serializers.URLField): | ||
| """URLField that returns None for empty strings""" |
There was a problem hiding this comment.
curious why this was necessary?
There was a problem hiding this comment.
Compromise between precommit insisting this field should be blankable but not nullable, and copilot review saying it should be null instead of a blank string.
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/10396
Description (What does it do?)
Screenshots (if appropriate):
How can this be tested?
OVS_API_BASE_URL=https://video-rc.odl.mit.eduin your backend .env filedocker compose updocker compose exec web python manage.py backpopulate_ovs_data fetchdocker compose exec web python manage.py backpopulate_ovs_data transcriptsVideodata should includecaption_urls(often empty) andcover_image_url(should not be null/blank for videos)