feat(sdk): first-class glossary term relations + system settings in Python & Java 2.0 SDKs#29788
feat(sdk): first-class glossary term relations + system settings in Python & Java 2.0 SDKs#29788harshach wants to merge 2 commits into
Conversation
…ython & Java 2.0 SDKs Wrap the glossary /relations endpoints (add/remove/graph/usage) and the glossaryTermRelationSettings system setting in both 2.0 SDKs so callers no longer drop to the raw HTTP escape hatch. Python (metadata.sdk): GlossaryTerms.add_relation/remove_relation/relations_graph/relation_type_usage plus a new Settings facade with an idempotent define_glossary_relation_type (JSON-Patch append, clobber-safe). Java (openmetadata-sdk): GlossaryTermService relation methods, a new SystemSettingsService exposed as client.settings(), and fluent GlossaryTerms.find(id).relateTo(to).as(type).apply(). GlossaryTermRelationsIT is ported off raw java.net.http onto the SDK. Relation-type registration uses JSON-Patch append (/relationTypes/-) rather than GET->append->PUT so concurrent callers never clobber each other's types on the shared glossaryTermRelationSettings global. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| """Unit tests for the glossary term relations + settings SDK facades.""" | ||
|
|
||
| import json | ||
| from unittest.mock import MagicMock |
There was a problem hiding this comment.
💡 Quality: Python test uses unittest.mock against pytest guidance
test_glossary_relations.py uses from unittest.mock import MagicMock. The Python/ingestion testing guidance is to use pytest with plain assert (which the file does correctly) but to avoid unittest mock styles where possible. Consider using pytest-mock's mocker fixture (or monkeypatch) instead of constructing MagicMock objects directly, to stay consistent with the project's preferred style.
Sources: ingestion/tests/unit/sdk/test_glossary_relations.py:4,16-30
Was this helpful? React with 👍 / 👎
|
🟡 Playwright Results — all passed (24 flaky)✅ 4086 passed · ❌ 0 failed · 🟡 24 flaky · ⏭️ 22 skipped
🟡 24 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
…ency docs Addresses code review on #29788. Python remove_relation/relations_graph now build query strings via urllib.parse.urlencode instead of raw f-strings, so relation-type names with reserved characters (space, &, #) cannot malform the URL. Java already encodes via RequestOptions + OkHttp addQueryParameter. Adds an encoding regression test. Clarifies define_glossary_relation_type / defineGlossaryRelationType docs: the name check makes registration idempotent for sequential use but is not atomic — concurrent registration of the same new name can duplicate, and the server does not dedupe relationTypes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the review — addressed in b48d003. 2. Query params not URL-encoded (Python) — fixed. 1. Idempotency TOCTOU — documented. Confirmed the server does not dedupe: 3. |
Code Review 👍 Approved with suggestions 2 resolved / 3 findingsImplements first-class SDK support for glossary term relations and system settings with safe, idempotent registration. Refactor the Python unit tests to use 💡 Quality: Python test uses unittest.mock against pytest guidance📄 ingestion/tests/unit/sdk/test_glossary_relations.py:4 📄 ingestion/tests/unit/sdk/test_glossary_relations.py:16-30
Sources: ingestion/tests/unit/sdk/test_glossary_relations.py:4,16-30 ✅ 2 resolved✅ Edge Case: Idempotency check on relation-type register still races
✅ Edge Case: Query params not URL-encoded in Python relation facades
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| public Settings defineGlossaryRelationType(GlossaryTermRelationType relationType) | ||
| throws OpenMetadataException { | ||
| Settings current = getGlossaryRelationSettings(); | ||
| Settings result = current; | ||
| if (!relationTypeExists(current, relationType.getName())) { | ||
| result = appendRelationType(relationType); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
defineGlossaryRelationType still reads the current settings, checks the name locally, and then sends an unconditional add /relationTypes/- patch. When two Java SDK callers register the same absent relation type at the same time, both can pass the local check and both can append the same value. The settings PATCH path does not deduplicate the array, so the global relation type list can still persist duplicate names even though this API is presented as idempotent.
| existing = {entry.get("name") for entry in cls.glossary_relation_types()} | ||
| result: Optional[dict[str, Any]] = None # noqa: UP045 | ||
| if name not in existing: | ||
| patch = [{"op": "add", "path": "/relationTypes/-", "value": relation_type}] | ||
| result = cls._get_rest_client().patch( | ||
| f"{_SETTINGS_ENDPOINT}/{GLOSSARY_TERM_RELATION_SETTINGS}", | ||
| data=json.dumps(patch), | ||
| ) |
There was a problem hiding this comment.
define_glossary_relation_type still builds existing from a separate settings read before sending an unconditional append patch. If two Python SDK clients register the same absent name concurrently, both can observe it as missing and both can append to /relationTypes/-. Because the settings PATCH path does not deduplicate appended relation types, the shared settings list can still end up with duplicate names.
Code Review 👍 Approved with suggestions 2 resolved / 3 findingsImplements first-class SDK support for glossary term relations and system settings with safe, idempotent registration. Refactor the Python unit tests to use 💡 Quality: Python test uses unittest.mock against pytest guidance📄 ingestion/tests/unit/sdk/test_glossary_relations.py:4 📄 ingestion/tests/unit/sdk/test_glossary_relations.py:16-30
Sources: ingestion/tests/unit/sdk/test_glossary_relations.py:4,16-30 ✅ 2 resolved✅ Edge Case: Idempotency check on relation-type register still races
✅ Edge Case: Query params not URL-encoded in Python relation facades
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Resolves #29789
What
Makes glossary term relations and the
glossaryTermRelationSettingssystem setting first-class in both 2.0 SDKs, so callers no longer drop to the raw HTTP escape hatch (client().ometa.clientin Python /getHttpClient().execute(...)in Java) for these operations. The underlying REST endpoints already exist onmain— this is a thin, ergonomic wrapper layer.API
Python (
from metadata.sdk import GlossaryTerms, Settings)Java
Design notes
define_glossary_relation_type/defineGlossaryRelationTyperegister a type via JSON-Patch append (add /relationTypes/-) rather than GET→append→PUT. A full PUT replaces the wholerelationTypeslist, so concurrent callers on the sharedglossaryTermRelationSettingsglobal can overwrite each other's types; append only touches the caller's own entry. Both are idempotent (skip when a type of the same name already exists).TermRelationpayload shape, and json-patch construction all move inside the SDK. In Java, aJsonNodePATCH body auto-selectsapplication/json-patch+json.GlossaryTermRelationsITnow drives the SDK methods instead of hand-rolledjava.net.httpcalls — the end-to-end proof.Tests
GlossaryTermRelationsMockTest(6) — mocks theHttpClientboundary and asserts the real URL, HTTP method, payload shape, DELETE query param, graph/usage parsing, and the append-vs-skip idempotency + JSON-patch structure.test_glossary_relations.py(7) — mocks the ometa REST boundary and asserts URL/payload/idempotency for the Python facades.Verification
openmetadata-sdkcompiles (main + test);GlossaryTermRelationsMockTest6/6 green;spotless:checkclean on both Java modules.ruff check+ruff format --checkclean.make generate) — runs in CI.openmetadata-servicebuild) — signature-matched to the verified SDK methods + import-clean; compiles in CI.🤖 Generated with Claude Code
Greptile Summary
This PR adds first-class glossary relation support to the Python and Java SDKs. The main changes are:
Confidence Score: 4/5
This is close, but the registration race should be fixed before merging.
openmetadata-sdk/src/main/java/org/openmetadata/sdk/services/system/SystemSettingsService.java; ingestion/src/metadata/sdk/entities/settings.py
Important Files Changed
Reviews (2): Last reviewed commit: "fix(sdk): URL-encode glossary relation q..." | Re-trigger Greptile
Context used (3)