feat(ingestion): Support status aspect patching with auto_status_aspect.py#18239
Conversation
github-actions
Bot
added
the
ingestion
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Connector Tests ResultsAll connector tests passed for commit To skip connector tests, add the Autogenerated by the connector-tests CI pipeline. |
maggiehays
added
the
needs-review
| from datahub.utilities.server_config_util import ( | ||
| RestServiceConfig, | ||
| get_gms_config, | ||
| ) |
There was a problem hiding this comment.
In general, we prefer imports at the top
Is there any reason for having these nested? circular deps doesn't seem an issue here
| # PATCH only sets removed=false without overwriting other | ||
| # fields (lifecycleStage, lifecycleState, lifecycleLastUpdated). | ||
| patch_builder = MetadataPatchProposal(urn) | ||
| patch_builder._add_patch( |
There was a problem hiding this comment.
patch_builder._add_patch(...) calls a private method from outside its class — it works, but it's a code smell.
Adding a patch helper or StatusPatchBuilder is probably out of scope for this PR, but could we leave a TODO comment for now?
There was a problem hiding this comment.
_gms_supports_status_patch() itself (cloud vs core thresholds, missing-config → False) isn't unit-tested — only the patch/upsert branch selection is. That gating logic is the crux of the change; a small test would be well spent.
maggiehays
added
pending-submitter-response
There was a problem hiding this comment.
LGTM. The import is at module scope now, the StatusPatchBuilder is a much cleaner way to express the patch than reaching into _add_patch directly, and the version-gating logic is well covered by the new tests.
Generated by Claude Code
3 participants
There's a bug in auto_status_aspect.py where we are always overwriting the status aspect. Instead this should be a patch MCP since there is other info in this aspect like lifecycleStage.
This supports PATCH for the status aspect in GMS and then checks the server version on the auto status aspect populator and uses patch if it's supported.