Add custom metadata schema registry table and validation - #2456
Merged
Conversation
Implements resolve_schemas() helper in datajunction_server/internal/custom_metadata.py that returns the most-specific active JSON Schema per key, using a specificity score (namespace=+2, node_type=+1) to pick the winning row when multiple rows match.
…rved-global resolution - Add three new ORM columns to CustomMetadataSchema: owner (str), updated_by_id (FK→users), reserved (bool, default False) with migration columns and FK constraint - Replace plain unique index on (key, node_type, namespace) with a COALESCE expression index on (key, coalesce(node_type,''), coalesce(namespace,'')) to correctly enforce uniqueness when node_type/namespace are NULL (Postgres < 15 compatible) - Update resolve_schemas so reserved global rows (node_type IS NULL, namespace IS NULL, reserved=True) always win over any more-specific scoped rows for the same key - Extend both test files with round-trip, default, reserved-wins, non-reserved-loses, and duplicate-global-integrity-error tests
…resolution coverage
…name test, clean docstring
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
Schema registrations are retired by stamping deactivated_at rather than by deleting the row, and every read path in the registry filters on deactivated_at IS NULL. The unique index over (key, node_type, namespace), however, covered tombstones too, so registering a key again in a scope where it had once been retired failed: the writer looked for a live row, found none, inserted, and collided with a row it could not see. A retired key was effectively unregisterable in that scope forever. Making the index partial on deactivated_at IS NULL lines the constraint up with what the readers consider to exist, while still rejecting two live rows for the same scope. The COALESCE expressions stay as they are so the index does not depend on Postgres 15 NULLS NOT DISTINCT. The migration that creates this table has not been released, so it is amended in place instead of being followed by a corrective revision.
Rebasing this branch carried the migration across without moving its `down_revision`, which still named the revision that was head when the branch was cut. Main has moved since, so the file forked the chain and alembic saw two heads -- leaving `test_migrations_are_current` unable to run at all, which is also why it stopped guarding ORM/migration agreement for this table. Re-pointed at the current head rather than reconciled with a merge revision. Nothing has landed on either side yet; a merge revision records two histories that both happened, and this is one branch that simply needs to sit after main.
`pyupgrade` runs as a blocking hook in CI over every file the branch touches, and it rewrites `datetime.timezone.utc` to `datetime.UTC` on 3.11 and above. The call sat in a test file neither of the earlier fixes touched, so running the hook over only the edited files kept missing it.
The model declared `id` and the two user references as plain `Mapped[int]`, which compiles to `INTEGER`, while the migration creates them as `BIGINT`. The guard test cannot see the difference: it runs autogenerate with `compare_type: False`, deliberately, to avoid false positives. So the drift was invisible and would have stayed that way. Nothing was broken by it -- Python reads either width the same -- but the two user columns reference `users.id`, which is `bigint`, so a narrower column here is a mismatch across the reference rather than a cosmetic one. `filterable` gains the `server_default` the migration already sets, so an insert that omits the column behaves the way the model says it will, matching `reserved` right beside it. And the dialect variants come off: this server runs on Postgres, so `json_schema` is simply `JSONB` and the integers are simply `BigInteger`, rather than each carrying a branch for a database nothing uses. Worth doing before this merges rather than after: the migration has not shipped, so the two can still be reconciled by editing the model. Once the table exists, changing a column type costs a migration of its own.
shangyian
force-pushed
the
dj/cm-phase-3-validation
branch
from
August 26, 2026 08:47
8dabd7a to
4abc3c5
Compare
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
custom_metadatais a free-form JSON bag on every node, and currently we don't check what goes into it. However, validating the schema per-key-and-namespace is useful for teams who want to register and validate schemas in custom metadata.This PR adds a registry where a JSON Schema can be declared for a single
custom_metadatakey, and validates writes against it. A schema is registered at some scope: globally, for a namespace, for a node type, or all of the above, and the most specific applicable schema wins for a given node. A key incustom_metadatawith no registered schema will not be validated.Resolution picks the most specific row per key, scoring +2 for a namespace and +1 for a node type, so the resolution order would be:
Namespace matching includes sub-namespaces, so a schema on
financealso governsfinance.reporting. The one exception is a global row marked as reserved: it always wins and no scoped row can shadow it.Test Plan
Deployment Plan
Adds one new table, but has no behavior change on its own. With an empty registry, validation is a no-op for every existing writer. The first behavioral change comes when a schema is registered, which isn't possible until the next PR.