[DEPR]: Support for XBlock Runtimes with raw string scope IDs

[kdmccormick]

In other words: Starting with XBlock 6.0.0, we will assume that XBlock Scope IDs are instances of OpaqueKey.

(Most if not all people can ignore this DEPR. Only operators with entirely custom XBlock Runtime implementations need to pay attention. We're actually not aware of any such custom Runtime implementations currently, so this DEPR is most likely just a formality.)

Proposal Date

Proposed 2024-08-27
Communicated 2024-09-02

Target Ticket Acceptance Date

2024-09-11

Earliest Open edX Named Release Without This Functionality

Sumac (master: Late Sept 2024)

Rationale

History

edx-platform was created circa 20211 and the XBlock framework was created circa 2012. Originally, those systems used carefully-formatted strings (str instances) to identify XBlocks in various contexts:

• course runs,
• usages of blocks across the site, and
• definitions of XBlocks content.

Circa 2014, in order to deal with the fragility and complexity of those strings in the light of a major overhaul of edx-platform's MongoDB schema (from "old" ModuleStore to "split" ModuleStore), we created the opaque-keys package. An OpaqueKey is a value object which identifies an XBlock scope and has a well-defined string representation; each subclass of OpaqueKey identifies a different kind of scope. Specifically, in this new system:

• Course runs are identified by CourseKeys (and, more generally, LearningContextKeys).
• Usages of blocks are identified by UsageKeys.
• Definitions of block content are identified by DefinitionKeys.

edx-platform was completely migrated over from string IDs to OpaqueKey IDs and has exclusively used opaque-keys for the past decade.

However, this migration was never represented in the XBlock framework. In theory, any object can be used as an XBlock scope ID. XBlock tests still use string IDs, as does the xblock workbench. In practice, though, edx-platform is the only production XBlock runtime that any of us are aware of, so in the real world XBlocks are all running with OpaqueKey scope IDs rather than string scope IDs.

Rationale for now assuming that scope IDs are OpaqueKeys

As an XBlock developer, it is confusing that the XBlock documentation makes no mention of OpaqueKeys, and it is strange and unhelpful that the XBlock Workbench identifies blocks in a way that is inconsistent from edx-platform.

Furthermore, we are adding type annotations to the XBlock package. The type annotations would be significantly less potent and instructive if they had to support string IDs: they would all be typed as object or Any, so edx-platform would need to disable mypy with # type: ignore wherever it treated an ID is an OpaqueKey. However, once we assume that scope IDs are all OpaqueKeys, we can annotate various XBlock API signatures with LearningContextKey, UsageKey, DefinitionKey, etc.. This will allow for stronger correctness checking in CI, better documentation for core and plugin developers, and more accurate code intelligence for developers using IDEs.

Removal

Beginning in XBlock 6.0.0:

• XBlock API calls may raise if given usage ids which are not instances of opaque_keys.edx.UsageKey.
• XBlock API calls may raise if given definition ids which are not instances of opaque_keys.edx.DefinitionKey.
• MemoryIdManager will generate instances of UsageKey and DefinitionKey, as appropriate, rather than str.

Replacement

N/A

Deprecation

In the interest of making it easy to update unit tests, we will make the XBlock API raise obvious assertion errors wherever the new scope-IDs-are-OpaqueKeys assumption is violated.

We do not plan to raise warnings ahead of time. We believe that only the XBlock Workbench (xblock-sdk) is in violation of this assumption, and Axim will take care of fixing it.

Migration

If any production XBlock Runtimes exist using string scope IDs, those IDs can be substituted with OpaqueKey instances whose __str__ methods generate identical scope IDs. We do not expect any such Runtimes to exist, though. If you know of one and need help understanding the migration, please reach out.

Additional Info

None

Task List

edx-platform repo

Constrain XBlock to < 6:

# Date: 2024-mm-dd
# Description: XBlock>=6 drops support for string scope IDs. We expect that this will
# not break any edx-platform app code, but we should smoke-test that expectation, and
# we may need to fix some unit tests that use string scope IDs.
# Ticket: https://github.com/openedx/XBlock/issues/784
XBlock[django]<6

XBlock repo

• Update unit tests to use OpaqueKeys instead of strings for scope IDs.
• Add assert statements to certain constructors to ensure that IDs are OpaqueKeys.
• Update MemoryIdManager to generate OpaqueKeys instead of strings.
• Update XBlock documentation as necessary.
• Release XBlock version 6.0.0.
• Remove this bit from xblock/core.py:

  XBlock/xblock/core.py

  Lines 442 to 444 in 19e7527

  	After https://github.com/openedx/XBlock/issues/708 is complete, we can assume that
  	`.scope_ids.usage_id` is always a `UsageKey`, and that this method will
  	always return a `LearningContextKey`.

• Either as part version 6.0.0 or in a follow-up as 6.1.0: Type-annotate the entire XBlock API.

xblock-sdk repo

• Update the workbench's XBlock Runtime to use string scope IDs rather than OpaqueKey scope IDs.

back to the edx-platform repo

• Update certain unit tests to use OpaqueKeys instead of strings for scope IDs.
• Ensure that the change to MemoryIdManager has not affected its usage in the Learning Core runtime for Content Libraries.
• Remove the constraint and upgrade to XBlock==6.0.0

[kdmccormick]

@bradenmacdonald we talked a few months ago about making this change. Here's a formal DEPR for it.

@ormsbee , my understanding and my research indicates that edx-platform will be completely unaffected by this. Let me know if you disagree.

@robrap , just a heads up that I plan to bring this up at the edx-platform maintenance WG meeting on Thursday. I'm going to propose that the 6-month waiting period be waived since this shouldn't impact 2U, or any other known site operator for that matter.

[robrap]

Thanks @kdmccormick. Thoughts:

1. I'd drop the "if necessary", and simply constrain XBlock to <6. You can do that as early as you wish, and if you use the updated style of constraint comment , it would be the best way to communicate this work without having to be concerned with timing between the library publish and upgrade work.
2. I agree that the 6-month waiting period could be waived, unless you learn about something that neither of us considered.
3. From an edx-platform perspective, hopefully your assumptions prove true and this is a smooth refactor.

[ormsbee]

@kdmccormick: I had totally forgotten this still existed. 😱 No objections.

[kdmccormick]

I'd drop the "if necessary", and simply constrain XBlock to <6. You can do that as early as you wish, and if you use the openedx/edx-platform#35055, it would be the best way to communicate this work without having to be concerned with timing between the library publish and upgrade work.

@robrap Sounds good. Task list updated 👍🏻

[kdmccormick]

This is now Accepted. I will work through the removal steps listed above, hopefully soon, definitely before the Sumac cutoff.