feat: generate user-facing write/read schema models + offline validation#1135
feat: generate user-facing write/read schema models + offline validation#1135dgarros wants to merge 22 commits into
Conversation
Deploying infrahub-sdk-python with
|
| Latest commit: |
d474ef8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://89863d14.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-user-schema-infp-234.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## infrahub-develop #1135 +/- ##
====================================================
- Coverage 83.98% 83.59% -0.39%
====================================================
Files 139 144 +5
Lines 14390 12745 -1645
Branches 2381 1858 -523
====================================================
- Hits 12085 10654 -1431
+ Misses 1654 1525 -129
+ Partials 651 566 -85
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 14 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://cdn.github.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
ce6e067 to
aa401e0
Compare
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 20 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="infrahub_sdk/schema/main.py">
<violation number="1" location="infrahub_sdk/schema/main.py:281">
P1: The public constructible write class `NodeSchema` no longer inherits `_SchemaKindMixin`, so it loses the runtime `.kind` property and all capability helpers (`supports_artifact_definition`, `supports_hierarchy`, etc.) that were previously available. The generated `NodeSchemaWrite` base does not provide a replacement `kind`, so existing code that constructs `NodeSchema` and accesses `.kind` will now raise `AttributeError`. Consider either restoring the `_SchemaKindMixin` inheritance to `NodeSchema` and `GenericSchema`, adding a `kind` computed field to the generated write base, or documenting this as an intentional breaking change if write models are no longer meant to expose these helpers.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
45502c5 to
622c56f
Compare
ajtmccarty
left a comment
ajtmccarty
left a comment
There was a problem hiding this comment.
the main outstanding issue for me is finding some way to create the SchemaAPI classes without Mixins that have TYPE_CHECKING conditionals in them. there are also some comments from cubic that might need addressing
| the derived capability helpers and declares ``kind`` for the type checker. | ||
| """ | ||
|
|
||
| if TYPE_CHECKING: |
There was a problem hiding this comment.
I'm surprised this is necessary. if _SchemaKindMixin is only used in classes inheriting from BaseNodeReadSchema / BaseNodeWriteSchema, then I would think that name, namespace, etc. fields on the BaseNode... class would cover this case.
A Mixin with a TYPE_CHECKING conditional inside of it feels like a kludge. is there some other way to handle this?
There was a problem hiding this comment.
Hey — I was surprised by this too, so I had Claude dig into it. Sharing what it found, because I actually think the current form is the right call.
Why the block is there: at runtime this is fine — every class the mixin lands on (NodeSchemaAPI, GenericSchemaAPI, ProfileSchemaAPI, TemplateSchemaAPI) inherits from a generated read model that extends BaseNodeSchemaRead, where name / namespace / kind / attributes / … actually live, so MRO resolves everything. The issue is purely static: the type checker analyzes _SchemaKindMixin in isolation at its definition, where it has no base that provides those attributes, so self.kind etc. come back "unresolved." The TYPE_CHECKING block is just the contract — "my host supplies these" — and emits zero runtime code. It's the documented idiom for typing mixins whose attributes come from the eventual host class.
(Detail: kind is declared as a @property, not kind: str, because on the generated base it's a read-only @computed_field — a plain annotation would model it as a settable field and clash in the MRO.)
Why not just rely on the fields from BaseNodeSchemaRead: that only works at runtime. The whole point of a mixin here is to share these helpers across four distinct generated read classes; the mixin has no single base to inherit the fields from at its own definition site, which is exactly what the checker looks at.
Why the alternatives are worse:
- Make each mixin subclass its generated base (so fields are inherited): this removes the block, but turns the plain, field-less mixins into full pydantic models with diamond inheritance, and couples each mixin to one specific generated base — losing the "compose onto any of them" property that motivated the mixin. It also drags back the field-collection subtleties (and the
kindcomputed-field clash) we deliberately kept out. Net negative. - Protocol-typed
selfon every method: doesn't actually remove the contract, just relocates it — and now every helper (including the@propertygetters) carries aself: SomeProtocolannotation. More noise, not less. - Generate the helpers into the read models: this behavior is SDK-specific (e.g.
kind == "CoreArtifactDefinition"), not derivable from the schema, so it would leak hand-written logic into files that are supposed to be pure generated schema.
So the TYPE_CHECKING block is the least-intrusive option and a recognized pattern rather than a one-off hack — I'd like to keep it as-is.
There was a problem hiding this comment.
This sounds like a contract without any verification. the _SchemaKindMixin class seems like it indicates that any class using this Mixin needs to define name, namespace, inherit_from, and kind instance variables or properties, but I don't think there is any checking that actually verifies this. for example if NodeSchemaAPI does not define name, then I don't think it would be identified by type-checking. if I am wrong about that, then I will approve this pattern, but I can't approve something that has 3 strikes against it
- it's a Mixin, which I consider a code smell
- it uses TYPE_CHECKING, which I consider a crutch that hides import errors regardless of where it is used
- it isn't actually type checked
I think the right approach is to make _SchemaKindMixin and _SchemaRelAttrMixin one base class for the Node/Generic/Profile/TemplateSchemaAPI classes, even if it uses "diamond inheritance"
| assert any("inherited" in message for message in result.messages), result.messages | ||
| # The message must locate the offending field within the payload. | ||
| assert any("nodes[0].attributes[0]" in message for message in result.messages), result.messages |
There was a problem hiding this comment.
I think these error assertions could be a little more restrictive. I think there should only be 1 error and it should be possible to assert more, or all, of its text
same for other error message assertions in this test file
| # cardinality is typed with the RelationshipCardinality enum (use_enum_values keeps the runtime | ||
| # value a plain string); a valid enum value must still validate. |
There was a problem hiding this comment.
# test string for RelationshipCardinality is valid
| assert any("not_a_field" in message for message in result.messages), result.messages | ||
|
|
||
|
|
||
| def test_raise_on_error_raises_value_error_naming_field() -> None: |
There was a problem hiding this comment.
looks very similar to test_out_of_enum_value_is_rejected_naming_field_and_value
622c56f to
d40aa55
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
d15ce11 to
02b53aa
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Add committed, generated write and read schema model variants under infrahub_sdk/schema/generated/. They are produced by the backend generator from the single source of truth in internal.py, filtered by a new field visibility axis (write ⊆ read ⊆ internal). The models are self-contained (pydantic + typing only) so they import with only the SDK installed, the write model retains extra="forbid", and constrained fields carry their allowed-value set as Literal[...] so the emitted JSON-schema is complete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ft guard [INFP-234] Add validate_schema() to validate a schema payload against the generated write models with only the SDK installed (no server): it returns a field-level verdict rejecting non-settable/unknown fields and out-of-enum values. Add SDK unit tests for offline validation and a drift guard that asserts the generated write/read models are present, carry the do-not-edit header, and satisfy the write(extra=forbid)/read-superset invariants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the offline write-contract validation so the attributes and relationships nested under extensions.nodes[*] are held to the same generated write models as node/generic-level ones. Previously only top-level nodes and generics were gated, letting read-level, unknown, and out-of-enum fields slip through on extension payloads. Add SDK offline tests covering extension attribute/relationship rejection with dotted error locations, plus breadth coverage for out-of-enum relationship cardinality/kind and read-level fields on relationships, generics, and nodes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…per-family Write/Read Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…f per-collection map Validate nodes and generics in one pass against the generated write document model; keep the separate extension gating (extension nodes are kind-only). Field-level dotted errors unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t[str, Any] The computed_attribute block is now a dedicated model (ComputedAttributeWrite/Read) with a Literal kind and extra=forbid, so the write contract for a computed attribute is explicit and its kind/unknown-field errors are caught with a field-level location. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…_attribute Replace the opaque dict[str, Any] sub-blocks in the generated write/read schema models with typed models: DropdownChoice, a plain union of the five attribute parameter shapes, and a kind-discriminated union for computed_attribute that enforces the jinja2_template/transform requirement natively. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the flat attribute write/read models, which typed parameters as a
plain union of every parameters shape, with a discriminated union on kind.
A shared AttributeSchemaBase carries every field except parameters, and each
variant narrows kind to the kinds sharing one parameters shape and carries
that parameters model, so a Text attribute no longer validates NumberPool
parameters. The public AttributeSchema{Write,Read} name becomes the union
alias.
Route offline validation of a single item through pydantic.TypeAdapter so a
union alias (which has no model_validate) validates like a plain model.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… models Extend the generated user-facing schema contract so it is complete: - Write variant gains NodeExtensionWrite / SchemaExtensionWrite (extra="forbid") and InfrahubSchemaWrite now carries an extensions field and forbids extra top-level keys, so the published write contract covers schema extensions. - Read variant gains read-only ProfileSchemaRead / TemplateSchemaRead so every read item is described by a generated model. - validate_schema now validates the whole root (nodes, generics, extensions) via InfrahubSchemaWrite in a single pass; the bespoke extension helper is retired while field-level dotted paths and the "(received: ...)" suffix are preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Constrained fields in the generated user-facing write/read models were rendered as inline Literals. Emit dedicated (str, Enum) classes into a new self-contained generated/enums.py and type the fields with them, matching the SDK's historical enum names. Every generated model gains use_enum_values=True so runtime values stay plain strings; attribute/computed-attribute discriminated unions use Literal[Enum.MEMBER] discriminators. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… [INFP-234] Retire the hand-maintained model bodies in ``infrahub_sdk/schema/main.py`` and re-base every public name on the generated write/read models plus hand-written behavior mixins, keeping import paths, names, methods and envelopes stable. - Enums (``AttributeKind``, ``BranchSupportType``, ``RelationshipCardinality``, ...) are re-exported from ``generated.enums`` under the historical names. - Behavior mixins carry the 15 attribute/relationship helpers, the ``kind`` / ``supports_*`` / hierarchy flags, ``cardinality_is_*`` and write ``convert_api``. - Read ``*API`` models stay concrete subclasses of the generated ``*Read`` models so ``isinstance`` keeps working; write ``NodeSchema``/``GenericSchema``/ ``RelationshipSchema`` subclass the generated write models. - A thin constructible ``AttributeSchema``/``AttributeSchemaAPI`` (on the shared write/read base) keeps ``AttributeSchema(name=..., kind=...)`` working; nodes narrow ``attributes``/``relationships`` to those variants and still validate into the strict generated discriminated union on dump. - Envelopes (``SchemaRoot``, ``SchemaRootAPI``, ``BranchSchema``) stay hand-written on the generated inner models. Breaking: ``AttributeKind.STRING`` removed; write-model defaults now match the server contract (relationship min/max_count 0, node branch "aware", generate_profile True, generate_template False) and reject unknown fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Enum-typed generated fields defaulted to the raw string value, which failed mypy/ty against the enum annotation. Emit the enum member (use_enum_values coerces to the value at runtime). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… [INFP-234] The enum-typed RelationshipSchemaWrite.cardinality field makes ty flag the deliberate raw-string construction used to prove pydantic's use_enum_values runtime coercion. Add a scoped ty: ignore so the intent-preserving test passes python-lint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kind (namespace+name) and hash (server-computed) are read-only. Expose kind as a pydantic computed_field and hash as a plain field on the generated read base node model, so node/generic/profile/template read models inherit both. Drop the hand-written kind property from the schema kind mixin and the hand hash fields from the API read models; the write NodeSchema/GenericSchema no longer carry the read-only kind mixin. Retype CoreNodeBase._schema to the read union so static _schema.kind access stays valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reading a locally-authored (write) schema needs `.kind` attribute access (e.g. the protocols CLI command). Expose `kind` as a plain property on the write node models — derived from namespace+name, like on read — but do not serialize it: on write it stays a property (not a `@computed_field`) so it never enters the payload, where `extra="forbid"` would reject it on the round-trip through the write/load contract. `hash` remains read-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…se [INFP-234] Picks up the upstream `ordered` attribute field (now classified write-visible) in the generated schema models and syncs protocols.py with the current backend core models. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ct [INFP-234] The /api/schema/load write contract rejects read-only and internal fields. Callers commonly build a payload from a schema read back from Infrahub or a full model dump, which carries those fields. schema.load()/check() now project each payload onto the generated write models before sending — dropping read-only/internal keys and resolving the per-kind attribute and computed-attribute discriminated unions — so such payloads load cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…INFP-234] Stripping read-only/internal fields client-side silently dropped fields the server is meant to reject (e.g. a node-only field on an extension), defeating the server's rejection contract and hiding user typos. The tolerate/reject decision now lives server-side in the schema-load endpoint instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…NFP-234] Replace the TYPE_CHECKING mixin contract on the *SchemaAPI models with a real base class that inherits the generated read models, so the shared fields and behavior are type-checked rather than merely declared. Node/Generic/Profile/ Template inherit (_SchemaNodeBase, <SpecificRead>); relationship cardinality helpers are inlined on RelationshipSchemaAPI (single consumer, and a relationship diamond would break MRO). Fix AttributeSchema to actually relax the inherited extra="forbid" (pydantic v2 merges subclass model_config), restoring lenient construction; strict rejection stays on the generated write union and the server contract. Align SchemaRoot with the generated write contract: expose extensions (SchemaExtensionWrite) in place of the dead node_extensions, and validate schema payloads against InfrahubSchemaWrite so the local check matches /api/schema/load. Consolidate the near-duplicate out-of-enum validation tests into one parametrized case table, and make the use_enum_values test able to detect a regression by asserting the runtime value is not an enum instance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… [INFP-234] Set extra="ignore" on the generated write and read schema models instead of the write-side extra="forbid". A submitted field that is not part of the write contract — read-level, internal, or a genuine typo — is now dropped silently rather than rejected, keeping schemas exported from Infrahub or hand-edited loadable. The read models tolerate additional fields returned by a newer server. Value validation is unchanged: unknown enum members, out-of-range constrained values, and missing required fields are still reported field-by-field. Update the offline-validation tests to assert non-write fields are tolerated and dropped on round-trip, and the generated-model tests to assert extra="ignore". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…P-234] Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6be132f to
c9f9f04
Compare
…d [INFP-234] The upstream IPAddress attribute-kind tests build a schema with kind="IPAddress", but this branch generates AttributeKind from the backend, which does not yet define an IPAddress attribute type. Skip until the backend adds it and the generated enum includes it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
Adds generated, user-facing write and read schema models to the SDK plus an offline
validate_schema()so a schema can be checked for correctness with no running Infrahub server. Part of the user-facing schema separation (opsmill/infrahub, INFP-234).Key Changes
infrahub_sdk/schema/generated/{write,read}.py— the write model is exactly what/api/schema/loadaccepts (constrained fields carry their allowed values asLiteral[...];extra="forbid"); read = write + visible-but-not-settable fields.validate_schema()(infrahub_sdk/schema/validate.py) — field-level offline validation with dotted error locations; also gates schemaextensions.protocols.py(pre-existing drift vs current backend schema).Related Context
Generated from the backend's schema definitions; consumed by the backend PR (opsmill/infrahub). Depends on nothing else in this repo.
Test Plan
14 offline-validation cases + generated-model guard pass, pydantic-only (no server).
Notes
Draft: the backend still ships a parallel hand-written model set; full consolidation of the SDK's hand-written schema models is tracked as a follow-up.
🤖 Generated with Claude Code
Summary by cubic
Adds generated user-facing write/read schema models and an offline validator so schemas can be checked without a server. This separates user schema from internal, reduces friction, and supports INFP-234.
New Features
infrahub_sdk/schema/generated/{enums,write,read}.py; exportInfrahubSchema{Write,Read}and enums; read addsProfile/Template, write includesextensions; both useextra="ignore".parametersandchoicesuse typed models; enums live ingenerated/enums.pywithuse_enum_values=True.validate_schema()validates full documents (nodes,generics,extensions) viaInfrahubSchemaWrite, returningSchemaValidationResultwith dotted paths; non-write/unknown fields are dropped while missing/invalid values are reported; includes a generated-model drift guard.Refactors
*SchemaAPIinherit the generated read models; enums re-exported;CoreNodeBase._schemaisMainSchemaTypesAPI.kind,hash) moved to generated read models; write nodes expose a non-serializing.kind;SchemaRootexposesextensionsand validates payloads againstInfrahubSchemaWrite.RelationshipSchemaAPI;AttributeSchemaconstruction is lenient (write unions remain strict);AttributeKind.STRINGremoved; defaults align with the server (relationshipmin_count/max_count=0; nodebranch="aware",generate_profile=True,generate_template=False); regeneratedprotocols.pywith new relationship managers.AttributeKindenum.Written for commit d474ef8. Summary will update on new commits.