Skip to content

Refactor video_shorts#2845

Merged
mbertrand merged 7 commits intomainfrom
mb/shorts_webhook_refactor
Jan 9, 2026
Merged

Refactor video_shorts#2845
mbertrand merged 7 commits intomainfrom
mb/shorts_webhook_refactor

Conversation

@mbertrand
Copy link
Member

@mbertrand mbertrand commented Jan 7, 2026

What are the relevant tickets?

Related to:

Description (What does it do?)

Refactors video_shorts model, serializers, views to align with changes to the dagster video_shorts pipeline in ol-data-platform:

  • rename youtube-specific variables
  • adjust for 2 thumbnails per video and changed metadata format sent by webhook

How can this be tested?

With your docker containers running, run the following from a python shell on your host OS (not docker):

import hmac
import hashlib
import json
import requests

data = {
  'video_id': '7f7d76532374f586', 
  'video_metadata': {
    'video_id': '7f7d76532374f586', 
    'video_url': '/shorts/7f7d76532374f586/7f7d76532374f586.mp4', 
    'title': "Is Moore's Law still true today?", 
    'published_at': '2025-11-06T00:00:00', 
    'thumbnail_small': {'width': 270, 'height': 480, 'url': '/shorts/7f7d76532374f586/7f7d76532374f586_small.jpg'}, 
    'thumbnail_large': {'width': 1080, 'height': 1920, 'url': '/shorts/7f7d76532374f586/7f7d76532374f586_large.jpg'}
  }, 
  'source': 'video_shorts'
}

secret = "please-change-this"
payload_string = json.dumps(data, separators=(",", ":"))
signature = hmac.new(secret.encode(), payload_string.encode(), hashlib.sha256).hexdigest()
headers = {
    "X-MITLearn-Signature": signature,
    "Content-Type": "application/json",
}

response = requests.post("http://open.odl.local:8063/api/v1/webhooks/video_shorts/", data=payload_string, headers=headers)
print(response)

To test the management command:

  • Set the following in your backend.local.env file to the same as RC:
AWS_ACCESS_KEY_ID=
AWS_SECRET_ACCESS_KEY=
AWS_STORAGE_BUCKET_NAME=ol-mitlearn-app-storage-rc

The openapi CI failure is due to breaking changes in the v0 video_shorts API, I think that is okay, it is not used outside mit-learn itself.

@github-actions
Copy link

github-actions bot commented Jan 7, 2026

OpenAPI Changes

Show/hide 26 changes: 14 error, 6 warning, 6 info
26 changes: 14 error, 6 warning, 6 info
error	[response-property-became-optional] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		the response property 'results/items/video_url' became optional for the status '200'

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		removed the required property 'results/items/thumbnail_height' from the response with the '200' status

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		removed the required property 'results/items/thumbnail_url' from the response with the '200' status

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		removed the required property 'results/items/thumbnail_width' from the response with the '200' status

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		removed the required property 'results/items/youtube_id' from the response with the '200' status

error	[response-property-became-optional] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		the response property 'video_url' became optional for the status '200'

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		removed the required property 'thumbnail_height' from the response with the '200' status

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		removed the required property 'thumbnail_url' from the response with the '200' status

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		removed the required property 'thumbnail_width' from the response with the '200' status

error	[response-required-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		removed the required property 'youtube_id' from the response with the '200' status

error	[new-required-request-parameter] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		added the new required 'query' request parameter 'video_metadata'

error	[new-required-request-property] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		added the new required request property 'video_metadata'

error	[new-required-request-property] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		added the new required request property 'video_metadata'

error	[new-required-request-property] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		added the new required request property 'video_metadata'

warning	[response-optional-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		removed the optional property 'results/items/description' from the response with the '200' status

warning	[response-optional-property-removed] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		removed the optional property 'description' from the response with the '200' status

warning	[request-parameter-removed] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		deleted the 'query' request parameter 'youtube_metadata'
		This is a warning because some apps may return an error when receiving a parameter that they do not expect. It is recommended to deprecate the parameter first.

warning	[request-property-removed] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		removed the request property 'youtube_metadata'

warning	[request-property-removed] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		removed the request property 'youtube_metadata'

warning	[request-property-removed] at head/openapi/specs/v1.yaml	
	in API POST /api/v1/webhooks/video_shorts/
		removed the request property 'youtube_metadata'

info	[response-optional-property-added] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		added the optional property 'results/items/thumbnail_large_url' to the response with the '200' status

info	[response-optional-property-added] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		added the optional property 'results/items/thumbnail_small_url' to the response with the '200' status

info	[response-required-property-added] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/
		added the required property 'results/items/video_id' to the response with the '200' status

info	[response-optional-property-added] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		added the optional property 'thumbnail_large_url' to the response with the '200' status

info	[response-optional-property-added] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		added the optional property 'thumbnail_small_url' to the response with the '200' status

info	[response-required-property-added] at head/openapi/specs/v0.yaml	
	in API GET /api/v0/video_shorts/{youtube_id}/
		added the required property 'video_id' to the response with the '200' status


Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@mbertrand mbertrand added the Needs Review An open Pull Request that is ready for review label Jan 7, 2026
@mbertrand mbertrand force-pushed the mb/shorts_webhook_refactor branch from 79406fa to 87532e4 Compare January 7, 2026 21:13
@abeglova abeglova assigned abeglova and unassigned abeglova Jan 8, 2026
Copy link
Contributor

@shanbady shanbady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some minor comments. otherwise looks good


settings.APP_BASE_URL = "https://learn.mit.edu/"
settings.VIDEO_SHORTS_S3_PREFIX = "youtube_shorts"
settings.VIDEO_SHORTS_S3_PREFIX = "shorts"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default in settings.py for VIDEO_SHORTS_S3_PREFIX has a trailing slash - should be one or the other

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setting is now irrelevant here anyway (it's only needed for the api.walk_video_shorts_from_s3 function) so I removed it here and elsewhere, and paramaterized prefix for one of the walk_video_shorts_from_s3 tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for consistency with how we instantiate boto resources elsewhere this should be instantiated via settings (which in turn should/does grab this value from the env)

s3 = boto3.resource(
        "s3",
        aws_access_key_id=settings.AWS_ACCESS_KEY_ID,
        aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY,
    )

@abeglova abeglova self-assigned this Jan 9, 2026
Copy link
Contributor

@abeglova abeglova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mbertrand mbertrand force-pushed the mb/shorts_webhook_refactor branch from df8e1ba to f881409 Compare January 9, 2026 15:48
@mbertrand mbertrand merged commit 669c918 into main Jan 9, 2026
12 of 13 checks passed
@mbertrand mbertrand deleted the mb/shorts_webhook_refactor branch January 9, 2026 20:34
This was referenced Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants