Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read support for v2 library content using openedx-learning models #33579

Closed

Conversation

ormsbee
Copy link
Contributor

@ormsbee ormsbee commented Oct 24, 2023

This is building XBlock read support on top of #33577 (which should be merged first)

Status

This PR is absolutely not merge-ready. It's a machete hack to try to quickly see what things will break and where we'll have to make changes. The real version would require a more graceful way to support running both blockstore-based and learning core-based models and switch between them. Also, I'm plugging straight into openedx-learning model queries right now, though they should be replaced with proper API calls (that don't exist yet).

Next Steps

  • Have a graceful way of selecting which backend it hits for a given library instead of the semi-manual switching I'm doing now.
  • Create real API calls in openedx-learning instead of digging into the models directly.
  • Tests.
  • Lots of general cleanup.
  • Figure out parent-child relationships, and to what extent we need to support them.

I'm going to copy a lot of code from Blockstore based things into Learning Core based things, instead of refactoring to extract common pieces–mostly because I eventually want to force the migration to happen, and things to cut over

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 27, 2023

FYI @bradenmacdonald, @kdmccormick, @connorhaugh: This one is not at all ready to be reviewed–it's just me hacking my way through and seeing what breaks.

Right now, static assets don't work at all, and I haven't implemented a real save_block functionality in the XBlock runtime (because I don't have a functioning UI to test that yet).

I think we can keep it pretty fast while ditching much of the caching related code.

@bradenmacdonald
Copy link
Contributor

I think we can keep it pretty fast while ditching much of the caching related code.

Yeah, ditch the caching if you can - it's only because pulling from S3 has such high latency.

@ormsbee ormsbee force-pushed the learning-core-libs-read-runtime branch from 7e74a21 to 7b82281 Compare November 16, 2023 20:22
@@ -527,3 +528,30 @@ def update_score(self, weighted_earned, weighted_possible, timestamp):

def __str__(self):
return str(self.usage_key)


class ContentLibraryLearningPackage(models.Model):
Copy link
Member

Choose a reason for hiding this comment

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

@ormsbee With Blockstore, a published library version can be identified by an int, corresponding to BundleVersion.version. That's what gets stored in LibraryContentBlock.source_library_version.

With this PR (which I know is very much a WIP), it looks like each component would have a version_num available (through PublishableEntity), but the library itself wouldn't--we'd need to look at the corresponding PublishLog.uuid for "published" versions and store that UUID in in LibraryContentBlock.source_library_version.

Does that sound right? Alternatively, would we want ContentLibraries themeslves (and courses, for that matter) to be PublishableEntities, making version_num available?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants