docs: add ADR for cross-instance taxonomy identity#656
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @mgwozdz-unicon! 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. DetailsWhere 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. |
mgwozdz-unicon
requested review from
bradenmacdonald,
jesperhodge,
kdmccormick and
ormsbee
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
I'm not totally opposed to using UUIDs here, but I don't think it's necessary and I do think you can achieve everything that you want by making an effort to make the default value of export_id "more unique" than it currently is.
| No stable, cross-instance identity exists for a ``Taxonomy`` or ``Tag`` today. ``Taxonomy.export_id`` | ||
| and ``Tag.external_id`` are both editable, instance-scoped identifiers designed for import-file | ||
| bookkeeping (see :ref:`openedx-tagging-adr-0006`), not for answering "does the target already have |
There was a problem hiding this comment.
Hmm, I guess it's not documented clearly but the goal of export_id is actually to identify taxonomies across different instances, exactly what you are requesting with this ADR. It is meant for answering "does the target already have this taxonomy."
See openedx/modular-learning#183
"As a content author, I want a consistent way to identify taxonomies, especially across instances".
| Add an immutable ``uuid`` field to ``Taxonomy``, generated at creation and preserved through | ||
| export/import. ``export_id`` cannot serve this purpose on its own: it is free text, chosen and | ||
| editable by whoever administers a taxonomy, so nothing guarantees that the same ``export_id`` on | ||
| two different instances refers to the same taxonomy, or that two different taxonomies on two | ||
| instances never happen to share one. A ``uuid`` is generated once, never touched by a person, and | ||
| so cannot collide or drift the way a human-chosen identifier can. ``export_id`` keeps its existing | ||
| role as the human-meaningful identifier; ``uuid`` adds the machine identity it was never designed | ||
| to provide. This follows the existing convention in this codebase of using ``uuid`` for a stable | ||
| external reference (see ``PublishableEntity.uuid``, :ref:`openedx-content-adr-0003`), applied here | ||
| to a model that currently lacks it. |
There was a problem hiding this comment.
I don't think a UUID is what we want.
If I set up the Lightcast Open Skills taxonomy on instance A, and someone else sets up the Lightcast Open Skills taxononomy on instance B, there should be some way to indicate that they're the same, and future exports/imports across the instances will handle that. But if they each get an immutable UUID at the time of original import, then there is no way to achieve that. Whereas with export ID, it's easy, you just make sure they both have the same export ID like io.lightcast.open-skills.
| ``Taxonomy``/``Tag`` could gain version history (e.g. via ``django-simple-history``, matching | ||
| ``CompetencyCriteria``), which would let drift between source and target be tracked precisely | ||
| rather than detected only as additive or not. Likely needed eventually, but not required for this | ||
| use case, since the additions-only reconciliation policy already satisfies it without history. |
There was a problem hiding this comment.
Please link to #455 which is our tentative plan for making taxonomies versioned.
| Two institutions may independently author what they each consider the same taxonomy on their own | ||
| instances, without ever having copied content between each other. Since a ``uuid`` establishes | ||
| identity only once two taxonomies have actually shared a copy operation, importing between them | ||
| today would create a second, unrelated taxonomy on the receiving side, not recognize them as the | ||
| same one. Automatic matching cannot safely resolve this: falling back to matching by name or | ||
| content similarity would reintroduce the false-positive risk ``uuid`` was introduced to avoid, two | ||
| instances with genuinely different taxonomies that happen to look similar would be silently | ||
| merged. The eventual resolution is a deliberate, human-initiated action: an administrator manually | ||
| reassigns one instance's taxonomy to adopt the other's ``uuid``, retroactively establishing shared | ||
| identity going forward. Not designed here, since it is a distinct, rare operation, orthogonal to | ||
| the copy-time behavior this decision covers. |
There was a problem hiding this comment.
This is what I was saying above, but it can be done more easily via export_id. It's awkward to compare and edit UUIDs.
Reuse the existing Taxonomy.export_id field for cross-instance identity instead of adding a new uuid field, per review feedback.
|
Thanks @bradenmacdonald , this makes sense, went with You're right that Additionally:
Let me know if I've missed anything. |
ormsbee
left a comment
ormsbee
left a comment
There was a problem hiding this comment.
I agree with @bradenmacdonald's review, and using export_id. It's similar to what we already do for things like courses and libraries, where we rely on meaningful keys.
One thing I want flag is that courses and libraries are implicitly namespaced to an org, which is something that we're not currently doing with taxonomies. We don't really have system vs. org-level libraries or courses. If there is ever a situation in which multiple orgs want to independently tweak or update the same logical taxonomy, we may want to consider some kind of namespacing conventions.
I added a note about this to the "Excluded from this decision" section to flag it for the future. |
jesperhodge
left a comment
jesperhodge
left a comment
There was a problem hiding this comment.
Some clarifications needed :)
| Context | ||
| ------- | ||
|
|
||
| Competency Based Education (CBE) will bring about the use case where, when course content is |
There was a problem hiding this comment.
Before this ADR uses "competency criteria," can we get one plain sentence defining it? Something like: "A competency criterion asserts that a piece of course content demonstrates a specific competency, which it identifies by pointing at a tag in a taxonomy." Right now CompetencyCriteria/CompetencyCriteriaGroup show up in parentheses on line 17 with zero setup, and this ADR lives in openedx_tagging, which explicitly isn't supposed to know about competencies. A reader coming from the tagging side needs to know why a criterion touches a taxonomy at all before the rest of the Context makes sense.
| it. Competency criteria (``CompetencyCriteria``, ``CompetencyCriteriaGroup``) are versioned via | ||
| ``django-simple-history`` per :ref:`openedx-learning-adr-0003`, but the taxonomies and tags they | ||
| reference are deliberately non-evaluative, unversioned display metadata. Copying a criterion | ||
| therefore also means resolving the taxonomy and tags it points to on the target. |
There was a problem hiding this comment.
| it. Competency criteria (``CompetencyCriteria``, ``CompetencyCriteriaGroup``) are versioned via | |
| ``django-simple-history`` per :ref:`openedx-learning-adr-0003`, but the taxonomies and tags they | |
| reference are deliberately non-evaluative, unversioned display metadata. Copying a criterion | |
| therefore also means resolving the taxonomy and tags it points to on the target. | |
| it. Each competency criterion (CompetencyCriteria) identifies the competency it evaluates by referencing a specific Tag in a Taxonomy. Criteria themselves are versioned via django-simple-history (see :ref:openedx-learning-adr-0003), but the taxonomy and tags they reference are deliberately not: they're non-evaluative display metadata that can change independently without creating a new version of the criteria pointing at them. Copying a criterion therefore also means resolving the taxonomy and tags it points to on the target, since the copy has to land somewhere with a matching tag for it to reference. |
| Copy semantics | ||
| ~~~~~~~~~~~~~~ | ||
|
|
||
| Competency criteria are copied **by reference**: the target's criteria are bound to a taxonomy |
There was a problem hiding this comment.
Does this actually work?
- A
CompetencyCriteriais indeed bound to a tag application that is bound to an object and a taxonomy, where the object is an object in e.g. the course that's being copied. Does a course copy copy the taxonomy, and do the tags resolve so that the object tag ids actually apply to the new taxonomy_id that has the same export_id? - A
CompetencyCriteriais also bound to aCompetencyCriteriaGroup. How is thisCompetencyCriteriaGroupcopied to the new course or content? - Do I understand by reference to mean a reference of the criteria to the export_id of a taxonomy? Or by reference to the competency criteria? In that case I'm confused who references them and how the reference could be resolved if we have a different environment with a different database.
- A
CompetencyCriteriaGroupis optionally bound to acourse_id. I assume that is a course run. I assume it actually has to have the course_id to apply to a course. Since aCompetencyCriteriabelongs to aCompetencyCriteriaGroupthat belongs to a course, wouldn't copying it by reference mean that it does not apply to the new course now?
There was a problem hiding this comment.
To clarify my point 1:
An object_tag encodes a course run in the object_id. So if we do a course re-run, for example, the object_tags reference the previous course run, which means that the new course has no tags. When tags are copied to the new course, the copy of the tags somehow needs to be resolved to the new object_ids of the course content, not the same as previously. I assume this is handled via this tags file in course imports - and we will now apply the same logic to re-runs and such - but can you confirm this assumption is true?
There was a problem hiding this comment.
I am not sure if this is handled at all in course re-runs today. (But it should be.) Some testing will be required.
|
Claude flagged this: |
Distinguishes CompetencyCriteria/CompetencyCriteriaGroup/ObjectTag recreation (copied by value, re-keyed to new object/course ids) from Taxonomy/Tag identity (never duplicated, resolved by reference via export_id). Also defines competency criterion in plain terms with a reference to ADR 0002, and ties "course" to CourseRun (openedx_catalog), reconciling with ADR 0002's course_id.
|
@jesperhodge, addressed your comments:
On the course_id inconsistency you flagged separately: checked both ADRs against the actual @bradenmacdonald your suspicion on the rerun/ObjectTag question was right, confirmed above. |
|
@mgwozdz-unicon @jesperhodge Note that there is code to duplicate tags when an XBlock is duplicated but reruns don't involve the We probably need to add a similar handler for the |
6 participants
uuidfield onTaxonomyso that competencycriteria copied along with course content (new course run, course export/import, course/library
copy) can be bound by reference to the correct taxonomy on the target instance, rather than an
export_id match that isn't reliable across instances.
only, apply it; if it includes any rename, reparent, or delete, refuse the import rather than
risk silently mutating taxonomy state that unrelated content on the target depends on.
copy, the newer
openedx_contentComponent/Container model, and full taxonomy/tag versioningare explicitly out of scope, noted as excluded or deferred with reasoning.