[2.0] fix(glossary): include cardinality on default term relation types - #33554
gh-bot-collate[bot] wants to merge 1 commit into
Conversation
…8376) * fix(glossary): include cardinality on default term relation types The system-seeded glossary term relation types (relatedTo, synonym, broader, narrower, partOf, hasPart, etc.) were created without a cardinality value, so the GET /v1/system/settings/glossaryTermRelationSettings response omitted the field for defaults while returning it for custom relations. The UI fell back to MANY_TO_MANY for every default and, on the next PUT, wrote that fallback back into the system relations - overriding any intended value. Set the expected cardinality on each default in SettingsCache and add a 2.0.2 data migration that backfills the field for installs that were already seeded without it. broader/narrower follow strict SKOS semantics (ONE_TO_MANY / MANY_TO_ONE - a term has at most one broader parent); all other system relations stay MANY_TO_MANY. * chore(migration): move glossary cardinality backfill to 2.0.3 The 2.0.2 / v202 migration slot was taken by search-settings repairs on main. Relocate the glossary term relation cardinality backfill to the next free version (2.0.3 / v203) so it runs after the existing 2.0.2 migrations without colliding with them. * fix(migration): cast settings json to jsonb on postgres in v203 backfill The glossary cardinality backfill bound the settings payload as a string into openmetadata_settings.json, which is jsonb on Postgres - Postgres refuses the implicit varchar->jsonb cast and the whole migration (and therefore TestSuiteBootstrap / every IT in the lane) aborted. Select the DB-specific UPDATE the way PiiRecognizerMigrationUtil / v1126 do: plain :json for MySQL, :json::jsonb for Postgres. * fix(glossary): seed system relation cardinality as unbounded MANY_TO_MANY Resolves the enum/bounds inversion flagged in review by not imposing any per-relation cardinality on the system defaults - the original bug was only that GET returned a null cardinality, not that hierarchy needed enforcing. - SettingsCache: broader/narrower now MANY_TO_MANY with null bounds, matching every other system default. GET returns a non-null cardinality and the UI no longer falls back to (and persists) a guessed value - with zero new enforcement, so existing terms with multiple broader parents keep working. - v203 migration: backfill null cardinality on system-defined relations to MANY_TO_MANY and derive bounds via the canonical normalize(), deleting the hand-written (inverted) bounds switch and per-relation map. - IT: assert exact cardinality (MANY_TO_MANY) and null sourceMax/targetMax for all 10 system relations, so an inverted or bounded value fails the test. (cherry picked from commit 4c33d0a)
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
✅ Playwright Results — workflow succeededValidated commit ✅ 766 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 57m 51s ⏱️ Max setup 3m 3s · max shard execution 17m 12s · max shard-job elapsed before upload 20m 41s · reporting 5s 🌐 201.40 requests/attempt · 2.51 app boots/UI scenario · 24.42% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| public void backfillGlossaryTermRelationCardinality() { | ||
| GlossaryTermRelationSettings settings = loadSettings(); | ||
| if (settings == null || settings.getRelationTypes() == null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The new backfill method combines loading, filtering, mutation, normalization, logging, and persistence in roughly 25 lines, and it exits through an early guard. The repository’s Java guide requires focused methods of about 15 lines or fewer and one trailing return per method. loadSettings also introduces an early return, and the new integration-test method exceeds the method-size guideline. This repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
anuj-kumary
added
the
safe to test
|
2 participants
Automated backport of #28376 to
2.0.4c33d0a228011ac34c5f07f901cace05a333cb89main.Opened from openmetadata-collate, which holds the credentials needed to open a PR here. It merges itself once every check passes — no review needed. If a check fails it stays open and the author of #28376 is asked to look.
resolver run
The backfill appears behaviorally safe, but the explicit Java method-structure requirement should be satisfied before merging.
Findings
Summary
This backport makes the ten built-in glossary relation types explicitly unbounded MANY_TO_MANY relations on fresh installations and upgrades existing settings whose system-defined relations still have null cardinality.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[2.0.3 migration runner] --> B[Load glossary relation settings] B --> C{System-defined and cardinality null?} C -->|No| D[Leave relation unchanged] C -->|Yes| E[Set MANY_TO_MANY] E --> F[Normalize sourceMax and targetMax to null] D --> G{Any relation changed?} F --> G G -->|Yes| H[Persist settings JSON] G -->|No| I[Skip update] H --> J[Server initializes SettingsCache] I --> JReviews (1) · Last reviewed commit: "fix(glossary): include cardinality on de..."