Repository navigation
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @ChrisChV! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
ChrisChV
changed the title
| 2. Thumbnail metadata lives platform-side, not in ``openedx_content`` | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| A new Django model, ``ImageMediaThumbnail``, owned by openedx-platform, is keyed to the **source image**, not to the File component — via a ``ForeignKey`` referencing the source image's ``ImageMedia`` entry (:ref:`openedx-content-adr-0013`), and records which :class:`Media` object is the current thumbnail for that source image, together with any other presentation-adjacent fields the platform needs for that asset. Consistent with Decision 1, this model is a plain, mutable Django row: creating a new thumbnail or changing these fields is a simple update, not a new version. |
There was a problem hiding this comment.
ImageMedia doesn't actually exist. I think I just wrote that as an illustrative example of something I'd like to have down the road.
Also, if it is keyed to ImageMedia, which would be keyed to the immutable Media, why would ImageMediaThumbnail be mutable?
There was a problem hiding this comment.
I also mentioned ImageMedia in #822 and I do think we should build it if we're building thumbnails.
|
|
||
| Creating or regenerating a thumbnail never creates a new version of the File component. The source image continues to be versioned normally, like any other File component content, but the thumbnail itself is not authored content and does not participate in that versioning. | ||
|
|
||
| 2. Thumbnail metadata lives platform-side, not in ``openedx_content`` |
There was a problem hiding this comment.
It's not clear to me why this should be platform-owned, as opposed to owned by core.
| An independent storage model with no relationship to ``Media`` | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| A platform-side model with its own image field and storage backend, entirely disconnected from ``openedx_content`` — the pattern ``edxval.VideoImage`` already uses for video thumbnails — was considered. It is a simpler design, but it forfeits the content-addressed deduplication that :class:`Media` already provides, and it does not follow the extension pattern that :class:`Media` explicitly documents for third-party apps. |
There was a problem hiding this comment.
Deduplication should be fairly straightforward to do with a disconnected system if we preserved Media's content-based file naming convention, and just appended whatever the resize params were. So the thumbnail file storage could be something like, "{file_namespace}/{original-media-hash}-{thumbnail-params}.webp"
We could even be more aggressive about it and dedupe thumbnails across the whole site, rather than worrying about file storage namespaces.
|
Another thought for your consideration: We could make an image resizing system that sits completely outside the LearningPackage infrastructure, supports lazy generation, but is also invoked during import/upload. Dedupe can still happen, since we know the source Media file names, and can make a convention of re-using that. Plus we can potentially get much more flexibility, if we ever want to let people tweak the resize parameters. We can even encode the resize parameters into the filename. So if the file exists, we send it back, and if not, we generate it. Image resizing comes up in the context of thumbnails, but it also comes up in a number of other places in courseware, like taking absurdly oversized course card images or HTMLBlock images and resizing them down to something much smaller to send over the wire. |
5 participants
Builds on #822
AI disclosure: I used Sonnet 5 to help with research.