Repository navigation
Resolve Opaque Key coherency issues #322
Description
Development notes
PRs
The OEP:
openedx-core PRs:
- feat!: Collection.key -> Collection.collection_code #542
- feat!: Component.local_key -> Component.component_code #544
- feat!: Add Container.container_code field #545
- feat!: Package and Entity
keysare now opaquerefs#546 - feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path #547
openedx-platform PR:
Follow-ups
- Opaque Key Simplification opaque-keys#411
- Consider moving opaque_keys in, as openedx_keys #437
- Add container_type to container's entity_refs #578
- Take
Thing | Thing.IDfor all openedx_content APIs #562 - Improve usability of created/created_by/set_at/set_by in the publishing API ? #549
- Opaque Key Simplification opaque-keys#411
Testing
TBC
AI Usage in PRs
I used Claude Code to help me write all the PRs. I developed a plan for the entire set of 5 changes, and then had it break the plan into 5 parts, one for each commit, before having it implement everything. I've reviewed each changeset by hand and tweaked it, both using Claude and on my own. All commits which Claude was involved in have co-author credit.
Original ticket:
Problems and questions
- Inconsistent usage of id, pk, uuid, slug, key throughout learning core
- Confusion between usage keys and our new suite of non-usage-key "keys" (ie, PublishableEntity.key)
- Lack of standard parsing functions for PublishableEntity.key
- Wasted time and energy marshaling between UsageKeys and PublishableEntity.keys
- Should PE keys be usage keys?
- Should we delete PE UUIDs?
Original issue
Working with Learning Core's authoring api is mostly a joy, except for the inconsistent usage of the terms "id", "key", and "slug". This applies to the LC-related objects and models in opaque-keys and edx-platform, as well.
I would propose standardizing on these meanings of each term for all new content-related code, and for any existing code which is not too burdensome to change:
*_idis an integer primary key.*_uuidis a UUID.*_slugis a URL-friendly string which identifies something within a local scope. On the Component model, we currently call this is a "local key".*_keyis a string (or an opaque-keys object) which uniquely identifies something across a whole Open edX instance. Can be composed of slugs, or of other slug-like things like a block/component type.
For example, one change I'd like to make is:
- On the model,
Collection.keyshould be renamedCollection.slug - The corresponding field on the opaque-keys object,
LibraryCollectionLoctator.collection_idshould be renamedLibraryCollectionLocator.slug. - Function arguments and dataclass members should be renamed accordingly.
Activity
ormsbee commentedon Jun 4, 2025
I actually don't think we should be storing site-global identifiers within a
LearningPackageat all, since that content could be moved to a different instance entirely. My initial thought was that all key/slug-like identifiers would be local, and that instance-unique identifiers would be derived from them. I'm actually coming to think that even assuming UUIDs would be globally unique was a mistake on my part.kdmccormick commentedon Jun 4, 2025
This makes sense.
I agree, but calling them "keys" seems confusing to me. The most common use of Learning Core will be as a backend for edx-platform learning content, which almost always uses "key" to mean a global identifier for a learning context or a usage. I found myself tripping over this while developing the Import API. If it weren't for the type hints, it would have been very challenging to keep straight.
kdmccormick commentedon Jun 4, 2025
Interesting. Just trying to understand this-- is this because we'd want to be able to entirely duplicate the entities of one LP into a new LP without worrying about conflicting identifiers, which would work with the keys/slugs but break with uuids?
Are we using the UUIDs at all? If not, and if we see no clear use for them, I wonder if it would make sense to remove them before anyone depends on them.
bradenmacdonald commentedon Jun 25, 2025
I've generally not used the UUIDs at all, and they seem kind of redundant given we have integer primary keys for intra-Learning Core relationships, and tend to use high-level keys (opaque keys) based on
key/slugfor everything else.bradenmacdonald commentedon Jun 25, 2025
Also worth noting that in the REST APIs I've generally tended to use only the opaque key, but call it
idbecause some frontend tooling expects / only works when you have a field calledid.See https://github.com/openedx/edx-platform/blob/3651b74738012686a66c311d82d65b5c8d4354d6/openedx/core/djangoapps/content_libraries/rest_api/serializers.py#L34-L39 for example.
We usually define a "write-only" field called
slugthat can be used to set theslug/keyon creation, but is not used when reading objects back from the API.We also have an old TODO here to rename a legacy
definition_idfield from the blockstore era toslug: https://github.com/openedx/edx-platform/blob/3651b74738012686a66c311d82d65b5c8d4354d6/openedx/core/djangoapps/content_libraries/rest_api/serializers.py#L196-L204The REST API also has an inconsistency with collections that I didn't catch in time. While the other objects (blocks, containers, etc.) all use opaque keys in their REST API URLs, e.g.
/api/xblock/v2/xblocks/lb:BradenX:LC1:problem:example-prolem@draft/fields/for an XBlock, the Collections API uses a pair ofkeyand the library opaque key, e.g./api/libraries/v2/lib:BradenX:LC1/collections/basic-collection/. This is despite us having aLibraryCollectionLocatorthat we could be using to be similar to all the other library APIs.[-]Straighten out "id", "key", and "slug"[/-][+]Key coherency[/+]kdmccormick commentedon Oct 15, 2025
Renaming this to "Key coherency" in order to capture the variety of discussions and issues around this
kdmccormick commentedon Oct 15, 2025
@ormsbee @bradenmacdonald I'm becoming increasingly convinced that this is a major issue with Learning Core as it stands today. Here's an instance where an experienced edx-platform CC is mixing up UsageKeys and PE keys while having to hand-roll logic to parse the latter and convert it to the former: openedx/openedx-platform#37405 (comment) (sorry to use you as an example if you're reading this Navin, my point is that the API is confusing, and if you made this mistake then many many devs will in the future :)
With OpaqueKeys, we get parsing and serialization, we get type checking, we get inheritance. We can say things like "a UsageKey is a ContextKey plus a block key", and that continues to work when we introduce new kinds of LearningContexts. We can coherently think and say things like "this should be a BlockKey, not a UsageKey" and this makes sense and is unambiguous. By going back to stringly-typed keys, we're throwing that all away.
I understand that we want LC to be generic, so we are hesitant to introduce a very Open-edX-ey thing (OpaqueKeys) to it. Still, could we introduce some structured key data types into learning core, so that we can parse/serialize them consistently and having coherent shared language?
bradenmacdonald commentedon Oct 15, 2025
With the XBlock API, there was a goal to keep the core API LMS-agnostic (because of Google Course Builder and other theoretical LMSs), but that never paid off in any way. Instead, the result was that the XBlock API was insufficient to do anything useful, and you had to supplement it with the un-specified, undocumented edx-platform XBlock extensions (the emergent API).
It seems we may now be doing something similar with Learning Core where you can't do much useful with the Core APIs that don't know anything about UsageKeys or libraries, so you have to use the high-level wrappers from edx-platform to do anything at all, which is opposite to the goal of only needing the Learning Core API to do useful things.
I think there are three core problems to solve that will unlock the desired simplicity and value:
Opaque Keys. I don't think anyone loves opaque keys. The name is awkward, the import syntax is long and hard to remember (
from opaque_keys.edx.locator import LibraryLocatorV2), the key-vs-locator distinction is confusing, and there's a ton of deprecated/legacy code within the key codebase. But, tthey provide a ton of benefits that Kyle outlined ("we get parsing and serialization, ... this makes sense and is unambiguous"). Because Learning Core has avoided opaque keys, we are "forced" to create wrapper APIs for everything in edx-platform, e.g. thecontent_librariesapp.I suggest we consider something like the following:
from openedx_learning.keys import CourseKey(instead offrom opaque_keys.edx.keys import CourseKey)Events. Because Learning Core doesn't emit events, it forces us to create/use high level wrapper APIs - see Fire events from
openedx_contentso that downstream effects can be handled in platform #462Users and Authentication. Because Learning Core is forbidden from asking questions about authorization, it forces us to create/use high-level wrapper REST APIs. I hope that openedx-authz could help solve this in the future, but I have no idea if that has been accounted for.
But even when all that is in place, I wonder if concepts like "content libraries" and "upstream/downstream syncing" also need to be moved into learning core, or can you do useful things with just "learning package" and "components" ?
ormsbee commentedon Oct 15, 2025
I'm going to avoid discussing (2) and (3) for now to keep this thread on topic, though I agree they are points that need more discussion.
I think I'm coming around to the idea of adding opaque-key-awareness in Learning Core, but I'd want to have it as a translation layer. I really do not want to store system-global UsageKeys because I think that will give us a lot of grief if we start storing varying Course Runs in Libraries. I don't want to get into a place where LC assumes that it can create and store a UsageKey based on the LearningPackage.key and the publishable entity key, when in fact the UsageKeys necessary to run courses will be constructed differently (i.e. UsageKeys can be M:1 to PublishableEntities).
navinkarkera commentedon Oct 16, 2025
@kdmccormick No worries, it is indeed confusing and in this specific case I was also lazy and did not check the actual column data before naming the variable, so that is my bad. I do agree that not having a standard way to parse entity keys to UsageKeys is not optimal.
[-]Key coherency[/-][+]Resolve Opaque Key coherency issues[/+]kdmccormick commentedon Mar 17, 2026
Here's my latest proposal: https://openedx.atlassian.net/wiki/spaces/COMM/pages/6027083777/Key+Coherency+for+openedx-core+v1.0
97 remaining items
feat!: Component.local_key -> Component.component_code (#544)
feat!: Add Container.container_code field
feat!: Add Container.container_code field (#545)
feat!: Package and Entity `keys` are now opaque `refs`
feat!: Package and Entity `keys` are now opaque `refs`
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path
feat!: Package and Entity `keys` are now opaque `refs` (#546)
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path (#547)
refactor: Upgrade to openedx-core 0.44.0 (for OEP-68) (#38402)
kdmccormick commentedon Apr 30, 2026
Completed in 0.44, and the remaining work is split out into follow-up tasks linked in the description.
refactor: Upgrade to openedx-core 0.44.0 (for OEP-68) (openedx#38402)
refactor: Upgrade to openedx-core 0.44.0 (for OEP-68) (openedx#38402)