Add the ability to export the schema in yaml#835
Open
BeArchiTek wants to merge 14 commits intostablefrom
Open
Conversation
- Strip relationship fields that match schema loading defaults (direction: bidirectional, on_delete: no-action, cardinality: many, optional: true, min_count: 0, max_count: 0) - Exclude branch from attribute/relationship dumps (inherited from node) - Ensure scalar fields appear before attributes/relationships lists Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
read_only was unconditionally excluded from attribute dumps; move it to _ATTR_EXPORT_DEFAULTS so it is stripped only when False (the default) and kept when True (computed/read-only attributes). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…export - Drop relationships with kind Group, Profile, or Hierarchy from export output — these are always auto-generated by Infrahub and must not be re-loaded manually (doing so caused validator_not_available errors for hierarchical generics) - For GenericSchemaAPI objects that had Hierarchy relationships, restore the `hierarchical: true` flag so the schema round-trips cleanly - Add read_only: false to _REL_EXPORT_DEFAULTS to stop it leaking into relationship output Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ints - generics key now appears before nodes in the exported YAML payload - uniqueness_constraints entries that are auto-generated from unique: true attributes (single-field ["<attr>__value"] entries) are removed on export; user-defined multi-field constraints are preserved Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deploying infrahub-sdk-python with
|
| Latest commit: |
32b52cf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6b037366.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://bkr-add-schema-exporter.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## stable #835 +/- ##
==========================================
- Coverage 80.38% 72.38% -8.01%
==========================================
Files 115 116 +1
Lines 9926 9964 +38
Branches 1515 1520 +5
==========================================
- Hits 7979 7212 -767
- Misses 1423 2246 +823
+ Partials 524 506 -18
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 31 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Move export helpers from ctl/schema.py to infrahub_sdk/schema/export.py so the conversion logic is reusable outside the CLI. Regenerate infrahubctl docs to include the new export subcommand. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
the
type/documentation
ogenstad
reviewed
Feb 19, 2026
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new schema export feature: a CLI command 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
infrahub_sdk/constants.py (1)
6-19: Consider usingfrozensetforRESTRICTED_NAMESPACES.As a constant used exclusively for membership tests (
in),frozensetgives O(1) lookup and prevents accidental mutation.♻️ Proposed change
-RESTRICTED_NAMESPACES: list[str] = [ - "Account", - "Branch", - "Builtin", - "Core", - "Deprecated", - "Diff", - "Infrahub", - "Internal", - "Lineage", - "Schema", - "Profile", - "Template", -] +RESTRICTED_NAMESPACES: frozenset[str] = frozenset({ + "Account", + "Branch", + "Builtin", + "Core", + "Deprecated", + "Diff", + "Infrahub", + "Internal", + "Lineage", + "Schema", + "Profile", + "Template", +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/constants.py` around lines 6 - 19, RESTRICTED_NAMESPACES is currently a mutable list but used only for membership checks; change its type to an immutable frozenset to prevent accidental mutation and get O(1) lookups by replacing the list literal with a frozenset literal (e.g., frozenset({...})) where RESTRICTED_NAMESPACES is defined in infrahub_sdk/constants.py and ensure any code that treats it as a sequence (indexing/ordering) is not relying on list behavior.infrahub_sdk/schema/__init__.py (1)
533-553:export()always fetches the full schema even whennamespacesis specified.The client-side namespace filtering in
_build_export_schemasis applied after fetching all schemas from the server. For large deployments, passing the namespace filter to_fetch(already supported via thenamespacesparameter ofInfrahubSchema.fetch) would reduce payload size. Not a bug, but worth considering for performance at scale.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` around lines 533 - 553, The export() method currently fetches the full schema then filters client-side; change it to pass the provided namespaces to the server fetch to reduce payload. Update the call in export (function export) from awaiting self.fetch(branch=branch) to await self.fetch(branch=branch, namespaces=namespaces) so server-side filtering is used before calling _build_export_schemas(schema_nodes=..., namespaces=namespaces); ensure the namespaces parameter shape (None or list[str]) matches InfrahubSchema.fetch's namespaces arg.tests/unit/sdk/test_schema_export.py (1)
19-97: Duplicated test data builders across both test files.
_BASE_NODE,_BASE_GENERIC,_schema_response, and the_make_*helpers are defined in near-identical form in bothtests/unit/sdk/test_schema_export.pyandtests/unit/ctl/test_schema_export.py. Extracting these into a sharedtests/helpers/schema.pymodule would eliminate the duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/sdk/test_schema_export.py` around lines 19 - 97, Duplicate test data and builders (_BASE_NODE, _BASE_GENERIC, _schema_response and helpers _make_node_schema, _make_generic_schema, _make_profile_schema, _make_template_schema) should be extracted into a single shared helper module and both test files should import them; create a new helpers module that exports those constants and factory functions, replace the duplicated definitions in tests/unit/sdk/test_schema_export.py and tests/unit/ctl/test_schema_export.py with imports from the new module, and update any references to use the imported symbols so tests still construct NodeSchemaAPI, GenericSchemaAPI, ProfileSchemaAPI, and TemplateSchemaAPI using the shared builders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrahub_sdk/ctl/schema.py`:
- Around line 217-219: The default export directory function
_default_export_directory currently returns a str while the export command's
directory parameter is annotated as Path; update _default_export_directory to
return a Path (e.g., wrap the formatted string with Path(...)) so the default
value matches the annotated type for export, and/or add a unit/integration test
that calls the export CLI without --directory to assert the resulting type is
Path and no AttributeError occurs (look for references to
_default_export_directory and the export command / directory parameter to modify
code and add tests).
---
Nitpick comments:
In `@infrahub_sdk/constants.py`:
- Around line 6-19: RESTRICTED_NAMESPACES is currently a mutable list but used
only for membership checks; change its type to an immutable frozenset to prevent
accidental mutation and get O(1) lookups by replacing the list literal with a
frozenset literal (e.g., frozenset({...})) where RESTRICTED_NAMESPACES is
defined in infrahub_sdk/constants.py and ensure any code that treats it as a
sequence (indexing/ordering) is not relying on list behavior.
In `@infrahub_sdk/schema/__init__.py`:
- Around line 533-553: The export() method currently fetches the full schema
then filters client-side; change it to pass the provided namespaces to the
server fetch to reduce payload. Update the call in export (function export) from
awaiting self.fetch(branch=branch) to await self.fetch(branch=branch,
namespaces=namespaces) so server-side filtering is used before calling
_build_export_schemas(schema_nodes=..., namespaces=namespaces); ensure the
namespaces parameter shape (None or list[str]) matches InfrahubSchema.fetch's
namespaces arg.
In `@tests/unit/sdk/test_schema_export.py`:
- Around line 19-97: Duplicate test data and builders (_BASE_NODE,
_BASE_GENERIC, _schema_response and helpers _make_node_schema,
_make_generic_schema, _make_profile_schema, _make_template_schema) should be
extracted into a single shared helper module and both test files should import
them; create a new helpers module that exports those constants and factory
functions, replace the duplicated definitions in
tests/unit/sdk/test_schema_export.py and tests/unit/ctl/test_schema_export.py
with imports from the new module, and update any references to use the imported
symbols so tests still construct NodeSchemaAPI, GenericSchemaAPI,
ProfileSchemaAPI, and TemplateSchemaAPI using the shared builders.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infrahub_sdk/ctl/schema.py (1)
247-247: Extract"1.0"to a named constant.The schema file format version is hardcoded inline. If it ever needs to change (or if other parts of the codebase need to reference the same version), hunting for the magic string will be error-prone. A module-level constant (e.g., in
infrahub_sdk/constants.py, which already definesRESTRICTED_NAMESPACES) makes the contract explicit.♻️ Proposed refactor
In
infrahub_sdk/constants.py(or a suitable constants module):+ SCHEMA_FILE_VERSION = "1.0"Then in
infrahub_sdk/ctl/schema.py:- from ..async_typer import AsyncTyper + from ..async_typer import AsyncTyper + from ..constants import SCHEMA_FILE_VERSION # add alongside existing imports- payload: dict[str, Any] = {"version": "1.0"} + payload: dict[str, Any] = {"version": SCHEMA_FILE_VERSION}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/ctl/schema.py` at line 247, The schema file version is hardcoded as "1.0" in the payload definition; extract it to a module-level constant (e.g., SCHEMA_FILE_VERSION) and reference that constant instead of the string literal. Add SCHEMA_FILE_VERSION = "1.0" to the existing constants module (infrahub_sdk/constants.py alongside RESTRICTED_NAMESPACES), then update the payload assignment in infrahub_sdk/ctl/schema.py (where payload: dict[str, Any] = {"version": "1.0"}) to use the imported SCHEMA_FILE_VERSION constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@infrahub_sdk/ctl/schema.py`:
- Around line 217-219: The _default_export_directory function is correct: ensure
it returns a pathlib.Path by keeping the return type Path and the f-string
naming (function _default_export_directory) that constructs
Path(f"infrahub-schema-export-{timestamp}"); no changes required—leave the
timestamp generation (datetime.now(timezone.utc).astimezone().strftime(...)) and
the Path(...) return as-is.
---
Nitpick comments:
In `@infrahub_sdk/ctl/schema.py`:
- Line 247: The schema file version is hardcoded as "1.0" in the payload
definition; extract it to a module-level constant (e.g., SCHEMA_FILE_VERSION)
and reference that constant instead of the string literal. Add
SCHEMA_FILE_VERSION = "1.0" to the existing constants module
(infrahub_sdk/constants.py alongside RESTRICTED_NAMESPACES), then update the
payload assignment in infrahub_sdk/ctl/schema.py (where payload: dict[str, Any]
= {"version": "1.0"}) to use the imported SCHEMA_FILE_VERSION constant.
Resolve conflicts: - Keep export command in ctl/schema.py (removed on stable, added on branch) - Accept deletion of tests/unit/ctl/test_schema_export.py (tests moved to tests/unit/sdk/) - Move RESTRICTED_NAMESPACES into schema/export.py (removed from constants.py on stable) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
infrahub_sdk/schema/__init__.py (3)
532-552:export()fetches the full schema even whennamespacesis provided;namespacesdocstring is incomplete.Two related concerns:
Performance:
self.fetch(branch=branch)is called without forwardingnamespaces, so the full schema is always retrieved from the server and the namespace filtering is done entirely in Python. For large schemas this unnecessarily transfers and parses all data when only a subset is needed. Passing a pre-filtered namespace list tofetch()would leverage server-side filtering.Docstring: The
namespacesparameter description is silent about restricted namespaces (Core, Builtin) always being excluded from the result, even when explicitly listed.♻️ Suggested optimisation for `InfrahubSchema.export`
async def export( self, branch: str | None = None, namespaces: list[str] | None = None, ) -> dict[str, dict[str, list[dict[str, Any]]]]: """Export user-defined schemas organized by namespace. Fetches all schemas from the server, filters out system types and restricted namespaces, and returns a dict keyed by namespace with ``"nodes"`` and ``"generics"`` lists of export-ready dicts. Args: branch: Branch to export from. Defaults to default_branch. - namespaces: Optional list of namespaces to include. If empty/None, all user-defined namespaces are exported. + namespaces: Optional list of namespaces to include. If empty/None, + all user-defined namespaces are exported. Restricted namespaces + (e.g. ``Core``, ``Builtin``) are always excluded even when + explicitly listed. Returns: Mapping of namespace to ``{"nodes": [...], "generics": [...]}``. """ branch = branch or self.client.default_branch - schema_nodes = await self.fetch(branch=branch) + # Pre-filter to avoid fetching namespaces that will be excluded anyway. + fetch_namespaces = ( + [ns for ns in namespaces if ns not in RESTRICTED_NAMESPACES] + if namespaces + else None + ) + schema_nodes = await self.fetch(branch=branch, namespaces=fetch_namespaces) return self._build_export_schemas(schema_nodes=schema_nodes, namespaces=namespaces)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` around lines 532 - 552, The export() implementation currently always calls self.fetch(branch=branch) which retrieves the full schema; change it to forward the namespaces argument to the server by calling self.fetch(branch=branch, namespaces=namespaces) so server-side filtering is used, then pass the returned schema_nodes into self._build_export_schemas as before; also update the export() docstring's namespaces description to explicitly state that Core and Builtin (restricted) namespaces are always excluded from the result even if listed.
796-816: Same performance and docstring gap asInfrahubSchema.exportabove.
self.fetch(branch=branch)is called without forwardingnamespaces, and the docstring omits the restricted-namespace exclusion caveat — apply the same fix as suggested for the async variant.♻️ Suggested fix for `InfrahubSchemaSync.export`
branch = branch or self.client.default_branch - schema_nodes = self.fetch(branch=branch) + fetch_namespaces = ( + [ns for ns in namespaces if ns not in RESTRICTED_NAMESPACES] + if namespaces + else None + ) + schema_nodes = self.fetch(branch=branch, namespaces=fetch_namespaces) return self._build_export_schemas(schema_nodes=schema_nodes, namespaces=namespaces)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` around lines 796 - 816, The export method currently calls self.fetch(branch=branch) without forwarding the namespaces filter and its docstring doesn't mention that restricted namespaces are excluded; update the implementation to call self.fetch(branch=branch, namespaces=namespaces) so the namespaces parameter is honored, and update the export docstring for InfrahubSchemaSync.export to explicitly state that restricted/system namespaces are excluded and that the optional namespaces argument will limit which user namespaces are exported.
123-151:_build_export_schemassilently drops explicitly requested restricted namespaces.When a caller passes e.g.
namespaces=["Core"], the method returns{}with no indication thatCorewas excluded because it is restricted. Theexport()docstring for thenamespacesparameter doesn't document this behavior either. Consider adding a note to both docstrings, or emitting a warning when a caller-supplied namespace overlaps withRESTRICTED_NAMESPACES.💡 Suggested docstring addition for `_build_export_schemas`
"""Organize fetched schemas into a per-namespace export structure. Filters out system types (Profile/Template), restricted namespaces, and optionally limits to specific namespaces. + Note: + Namespaces listed in ``RESTRICTED_NAMESPACES`` (e.g. ``Core``, + ``Builtin``) are always excluded, even when they appear in + ``namespaces``. + Returns: Mapping of namespace to ``{"nodes": [...], "generics": [...]}``. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` around lines 123 - 151, The _build_export_schemas function currently drops caller-supplied restricted namespaces silently; update _build_export_schemas to detect intersection = set(namespaces) & RESTRICTED_NAMESPACES and emit a clear warning (e.g., warnings.warn) listing the restricted namespace(s) being ignored before proceeding, and update the export() docstring and _build_export_schemas docstring to note that RESTRICTED_NAMESPACES are always excluded and that a warning will be emitted when the caller requests them; reference the _build_export_schemas function, RESTRICTED_NAMESPACES constant, and the export() docstring so reviewers can find the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@infrahub_sdk/schema/__init__.py`:
- Around line 532-552: The export() implementation currently always calls
self.fetch(branch=branch) which retrieves the full schema; change it to forward
the namespaces argument to the server by calling self.fetch(branch=branch,
namespaces=namespaces) so server-side filtering is used, then pass the returned
schema_nodes into self._build_export_schemas as before; also update the export()
docstring's namespaces description to explicitly state that Core and Builtin
(restricted) namespaces are always excluded from the result even if listed.
- Around line 796-816: The export method currently calls
self.fetch(branch=branch) without forwarding the namespaces filter and its
docstring doesn't mention that restricted namespaces are excluded; update the
implementation to call self.fetch(branch=branch, namespaces=namespaces) so the
namespaces parameter is honored, and update the export docstring for
InfrahubSchemaSync.export to explicitly state that restricted/system namespaces
are excluded and that the optional namespaces argument will limit which user
namespaces are exported.
- Around line 123-151: The _build_export_schemas function currently drops
caller-supplied restricted namespaces silently; update _build_export_schemas to
detect intersection = set(namespaces) & RESTRICTED_NAMESPACES and emit a clear
warning (e.g., warnings.warn) listing the restricted namespace(s) being ignored
before proceeding, and update the export() docstring and _build_export_schemas
docstring to note that RESTRICTED_NAMESPACES are always excluded and that a
warning will be emitted when the caller requests them; reference the
_build_export_schemas function, RESTRICTED_NAMESPACES constant, and the export()
docstring so reviewers can find the changes.
ogenstad
reviewed
Feb 20, 2026
Contributor
ogenstad
left a comment
ogenstad
left a comment
There was a problem hiding this comment.
I've added some comments to this. If required for a project we can go forward with this. However the format change I mention to the .export() method would be considered a breaking change when we do that.
In order to allow for easier refactoring it would be good with an integration test where we loaded a schema to Infrahub and then exported it again to then compare the schema that we just loaded with what we got out from infrahub using infrahubctl.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
infrahub_sdk/schema/__init__.py (1)
564-566:⚠️ Potential issue | 🟠 MajorCache poisoning when
namespacesfilter is used
fetch()defaults topopulate_cache=True. Whenexport(namespaces=["Infra"])is called, the cache for that branch is overwritten with only the filtered namespace's schemas. Any subsequent call toall(branch=branch)withoutrefresh=Truewill silently return this truncated map.
get()recovers (re-fetches when the kind is absent), butall()does not—it returns whatever is in the cache as-is. Sinceexportis a read/emit operation, it should not mutate the shared cache.🐛 Proposed fix
- schema_nodes = await self.fetch(branch=branch, namespaces=namespaces) + schema_nodes = await self.fetch(branch=branch, namespaces=namespaces, populate_cache=False)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` around lines 564 - 566, The export flow currently calls fetch(...) which defaults to populate_cache=True and thus overwrites the branch cache when a namespaces filter is used; update the export implementation (the code that calls fetch before _build_export_schemas) to call fetch with populate_cache=False (i.e., fetch(branch=branch, namespaces=namespaces, populate_cache=False)) so the shared cache for the branch is not mutated by export; keep _build_export_schemas and other behaviors intact so get() and all() continue to rely on the real cache unless explicitly refreshed.
🧹 Nitpick comments (1)
infrahub_sdk/schema/__init__.py (1)
25-25:schema_to_export_dictpromoted to public API surfaceAdding
schema_to_export_dictto__all__makes it part of theinfrahub_sdk.schemapublic contract. The existing review comment on the return type (dict[str, Any]) already flags that a proper typed object would be preferable. Exporting the raw helper now makes it harder to evolve that API later without a breaking change.Also applies to: 68-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` at line 25, schema_to_export_dict was added to the module public API via __all__, which exposes a raw helper that is hard to evolve; remove schema_to_export_dict from the exported surface (leave RESTRICTED_NAMESPACES public) and either: 1) keep the helper private (rename or not include it in __all__) so callers use a stable public API, or 2) introduce and export a typed abstraction (e.g., ExportedSchema / build_exported_schema factory) that returns a well-typed object instead of dict[str, Any]; update any internal imports to reference the private helper or the new typed API (symbols: schema_to_export_dict, RESTRICTED_NAMESPACES, __all__).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrahub_sdk/schema/__init__.py`:
- Around line 832-834: The sync export path is poisoning the branch cache
because InfrahubSchema.fetch() defaults to populate_cache=True; in the sync
export method shown (where branch = branch or self.client.default_branch,
schema_nodes = self.fetch(branch=branch, namespaces=namespaces), return
self._build_export_schemas(...)) call fetch with populate_cache=False so that a
namespace-filtered fetch does not overwrite the branch cache; update the fetch
invocation to pass populate_cache=False while still passing branch and
namespaces and then call _build_export_schemas as before.
- Around line 141-144: The warning currently uses warnings.warn(...,
stacklevel=2) in _build_export_schemas which attributes the warning to the SDK's
export() call instead of the caller; update the stacklevel to 3 in the
warnings.warn invocation so the warning points to the user code that called
export() (locate the warnings.warn call inside
_build_export_schemas/export-related logic and change stacklevel=2 to
stacklevel=3).
---
Duplicate comments:
In `@infrahub_sdk/schema/__init__.py`:
- Around line 564-566: The export flow currently calls fetch(...) which defaults
to populate_cache=True and thus overwrites the branch cache when a namespaces
filter is used; update the export implementation (the code that calls fetch
before _build_export_schemas) to call fetch with populate_cache=False (i.e.,
fetch(branch=branch, namespaces=namespaces, populate_cache=False)) so the shared
cache for the branch is not mutated by export; keep _build_export_schemas and
other behaviors intact so get() and all() continue to rely on the real cache
unless explicitly refreshed.
---
Nitpick comments:
In `@infrahub_sdk/schema/__init__.py`:
- Line 25: schema_to_export_dict was added to the module public API via __all__,
which exposes a raw helper that is hard to evolve; remove schema_to_export_dict
from the exported surface (leave RESTRICTED_NAMESPACES public) and either: 1)
keep the helper private (rename or not include it in __all__) so callers use a
stable public API, or 2) introduce and export a typed abstraction (e.g.,
ExportedSchema / build_exported_schema factory) that returns a well-typed object
instead of dict[str, Any]; update any internal imports to reference the private
helper or the new typed API (symbols: schema_to_export_dict,
RESTRICTED_NAMESPACES, __all__).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrahub_sdk/schema/__init__.pytests/unit/sdk/test_schema_export.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/sdk/test_schema_export.py
- Fix import ordering in test_schema_export.py (infrahub_sdk.schema above TYPE_CHECKING) - Use clients fixture + client_types pattern matching existing test conventions - Fix warnings.warn stacklevel from 2 to 3 for correct caller attribution - Pass populate_cache=False in both async/sync export to prevent cache poisoning Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infrahub_sdk/schema/__init__.py (1)
542-566: Consider reducing duplication between async and syncexport().The body of
InfrahubSchema.exportandInfrahubSchemaSync.exportis identical except forawait. This pattern of duplicating method bodies is already established in the codebase (e.g.,fetch,all,get), so this is consistent, but if a third call-site is added to_build_export_schemasin the future this will be a common source of drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/schema/__init__.py` around lines 542 - 566, Extract the shared logic from InfrahubSchema.export and InfrahubSchemaSync.export into a single private helper (e.g., _export_impl) that takes branch and namespaces and returns the dict by calling fetch (or the sync fetch equivalent) and _build_export_schemas; then have InfrahubSchema.export await self._export_impl(...) and InfrahubSchemaSync.export call self._export_impl(...) synchronously. Ensure helper references the same symbols used now: branch defaulting to self.client.default_branch, fetch, and _build_export_schemas so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@infrahub_sdk/schema/__init__.py`:
- Around line 542-566: Extract the shared logic from InfrahubSchema.export and
InfrahubSchemaSync.export into a single private helper (e.g., _export_impl) that
takes branch and namespaces and returns the dict by calling fetch (or the sync
fetch equivalent) and _build_export_schemas; then have InfrahubSchema.export
await self._export_impl(...) and InfrahubSchemaSync.export call
self._export_impl(...) synchronously. Ensure helper references the same symbols
used now: branch defaulting to self.client.default_branch, fetch, and
_build_export_schemas so behavior remains identical.
…antic models Addresses PR review feedback: export() now returns a proper SchemaExport object instead of dict[str, dict[str, list[dict[str, Any]]]], making the API easier to understand and use. Includes .to_dict() for serialization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ogenstad
approved these changes
Feb 24, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
2 participants
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.
Fixes #151
Summary by CodeRabbit
New Features
Library
Documentation
Tests
Chores