feat!: use replace_urls service instead of runtime property [BD-13] - #318
Conversation
|
Thanks for the pull request, @Agrendalath! When this pull request is ready, tag your edX technical lead. |
|
Thanks for the pull request, @Agrendalath! This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate. |
21dd713 to
806cf43
Compare
replace_urls service instead of runtime property [BD-13]replace_urls service instead of runtime property [BD-13]
| from .drag_and_drop_v2 import DragAndDropBlock | ||
|
|
||
| __version__ = "3.1.3" | ||
| __version__ = "3.2.0" |
There was a problem hiding this comment.
Maple is an old release, so I didn't bother bumping the major version number. Let me know if we should do this, though.
|
@Agrendalath Since we are breaking the compatibility with Maple, and |
|
👍 Other than this comment, everything else looks fine.
|
BREAKING CHANGE: This breaks Maple compatibility.
806cf43 to
62ab205
Compare
|
@Agrendalath 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
@Agrendalath 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
BREAKING CHANGE: This breaks Maple compatibility.
Using the property of
runtimeis deprecated. It also breaks Block Structures when the ModuleSystem is unified with DescriptorSystem.Supporting information
Related PR: openedx/openedx-platform#31472
Testing instructions (optional)
block_idand run the following snippet:/dashboard. This will trigger the code to find the resume URL and attempt to re-generate Block Structures.Deadline
Before openedx/openedx-platform#31472.
Author's notes
A quick summary: the Block Structures are used for caching the structure of the Course Outline page (stored as files in S3). The caching operation has a limited runtime for XBlocks (it does not include all available runtime services). However, when the completion tracking is enabled, this still needs to generate student-specific data for the most recently completed XBlock to perform some calculations. There is a lot of redundancy and a massive room for optimizations, but there are enough layers to make a whole new epic of out debugging it.
Generating student data of DnDv2 replaces static links, but the relevant service is unavailable. Therefore, we ignore this step - it does not affect the result. As for why this is happening after merging the ModuleSystem and DescriptorSystem - previously, the caching operation was relying directly on the DescriptorSystem, which didn't have the replace_urls attribute. We still need to keep it in the ModuleSystem for backward compatibility. As the ModuleSystem and DescriptorSystem become the same class in the mentioned PR, it will now have the
replace_urlsattribute, which makes the logic in DnDv2 invalid.We could do a few different things - e.g., move the replacing static links operation to another part of the DnDv2 (or make it lazy), but this would break tons of tests, making it a more significant change. Given the limited time and budget here, this seemed like the most straightforward fix to me.