Skip to content

Add allow_upsert option to LineDelimitedJSONImporter#765

Open
minitriga wants to merge 3 commits intostablefrom
atg-20260122-ctl-load
Open

Add allow_upsert option to LineDelimitedJSONImporter#765
minitriga wants to merge 3 commits intostablefrom
atg-20260122-ctl-load

Conversation

@minitriga
Copy link
Copy Markdown
Contributor

@minitriga minitriga commented Jan 22, 2026

…n; enhance handling of RelatedNode inputsAdd allow_upsert option to LineDelimitedJSONImporter and load function; enhance handling of RelatedNode inputs

Summary by CodeRabbit

  • New Features

    • Added an allow_upsert option to the import command to control upsert behavior during imports.
  • Improvements

    • More robust handling and serialization of related-node-like objects to ensure IDs are used where appropriate.
    • Reduced duplicate relationship entries and simplified relationship detection for exports.
    • Improved consistency when importing existing records.
  • Tests

    • Added and updated tests covering upsert imports and related-node data handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 22, 2026

Walkthrough

This pull request wires a new boolean allow_upsert option from the CLI load command into the importer and LineDelimitedJSONImporter, exposing importer-level control over upsert behavior. It adjusts GraphQL rendering to serialize the id of RelatedNode-like objects instead of the object. RelatedNodeBase.init now accepts another RelatedNodeBase as input by converting it to a dict with id, hfid, and __typename. The JSON exporter renames and refactors a helper to identify_many_relationships. Tests were added/updated to cover these changes.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add allow_upsert option to LineDelimitedJSONImporter' accurately describes the main change: introducing a new allow_upsert parameter throughout the import pipeline (CLI, importer initialization, and JSON importer). While the PR also includes enhancements to RelatedNode handling and GraphQL rendering, the primary and most prominent change is the allow_upsert option addition.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Jan 22, 2026

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 072eeb6
Status: ✅  Deploy successful!
Preview URL: https://43c83d6a.infrahub-sdk-python.pages.dev
Branch Preview URL: https://atg-20260122-ctl-load.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 22, 2026

Codecov Report

❌ Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/transfer/exporter/json.py 16.66% 5 Missing ⚠️
infrahub_sdk/transfer/importer/json.py 0.00% 2 Missing ⚠️
infrahub_sdk/graphql/renderers.py 75.00% 0 Missing and 1 partial ⚠️

❗ There is a different number of reports uploaded between BASE (06577ba) and HEAD (072eeb6). Click for more details.

HEAD has 8 uploads less than BASE
Flag BASE (06577ba) HEAD (072eeb6)
python-3.11 2 1
python-3.10 2 1
python-3.12 2 1
python-filler-3.12 2 1
python-3.14 2 1
python-3.13 2 1
integration-tests 2 0
@@            Coverage Diff             @@
##           stable     #765      +/-   ##
==========================================
- Coverage   80.77%   72.93%   -7.85%     
==========================================
  Files         132      119      -13     
  Lines       10999    10340     -659     
  Branches     1681     1552     -129     
==========================================
- Hits         8884     7541    -1343     
- Misses       1566     2285     +719     
+ Partials      549      514      -35     
Flag Coverage Δ
integration-tests ?
python-3.10 51.85% <42.85%> (-1.72%) ⬇️
python-3.11 51.85% <42.85%> (-1.72%) ⬇️
python-3.12 51.87% <42.85%> (-1.70%) ⬇️
python-3.13 51.85% <42.85%> (-1.72%) ⬇️
python-3.14 53.57% <42.85%> (-1.60%) ⬇️
python-filler-3.12 24.03% <0.00%> (+1.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/ctl/importer.py 57.14% <ø> (ø)
infrahub_sdk/node/related_node.py 84.48% <100.00%> (-7.96%) ⬇️
infrahub_sdk/graphql/renderers.py 89.65% <75.00%> (-3.12%) ⬇️
infrahub_sdk/transfer/importer/json.py 15.38% <0.00%> (-61.38%) ⬇️
infrahub_sdk/transfer/exporter/json.py 20.43% <16.66%> (-51.45%) ⬇️

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@minitriga minitriga force-pushed the atg-20260122-ctl-load branch from ce6441e to f0e0140 Compare April 20, 2026 14:29
@minitriga minitriga requested a review from a team as a code owner April 20, 2026 14:29
@github-actions github-actions Bot added the type/documentation Improvements or additions to documentation label Apr 20, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 9 files

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="docs/docs/infrahubctl/infrahubctl-load.mdx">

<violation number="1" location="docs/docs/infrahubctl/infrahubctl-load.mdx:20">
P3: Don't hand-edit this generated CLI doc; update the source definition and regenerate the MDX instead.</violation>
</file>

<file name="infrahub_sdk/transfer/exporter/json.py">

<violation number="1" location="infrahub_sdk/transfer/exporter/json.py:56">
P1: This now exports one-way `many` relationships into `relationships.json`, but the importer treats that file as many-to-many-only and reapplies links, causing duplicate relationship updates.</violation>
</file>

<file name="infrahub_sdk/node/related_node.py">

<violation number="1" location="infrahub_sdk/node/related_node.py:52">
P2: Preserve `kind` when reusing an existing RelatedNode; otherwise HFID-backed references lose identifying data.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

# Record the relationship only if it's not known in one way or another
# This avoids duplicating bidirectional many-to-many relationships
if not forward and not backward:
many_relationship_identifiers[node_schema.kind, relationship.peer] = relationship.identifier
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This now exports one-way many relationships into relationships.json, but the importer treats that file as many-to-many-only and reapplies links, causing duplicate relationship updates.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/transfer/exporter/json.py, line 56:

<comment>This now exports one-way `many` relationships into `relationships.json`, but the importer treats that file as many-to-many-only and reapplies links, causing duplicate relationship updates.</comment>

<file context>
@@ -34,31 +34,26 @@ def wrapped_task_output(self, start: str, end: str = "[green]done") -> Generator
+                # Record the relationship only if it's not known in one way or another
+                # This avoids duplicating bidirectional many-to-many relationships
+                if not forward and not backward:
+                    many_relationship_identifiers[node_schema.kind, relationship.peer] = relationship.identifier
 
         return many_relationship_identifiers
</file context>
Suggested change
many_relationship_identifiers[node_schema.kind, relationship.peer] = relationship.identifier
has_many_peer = any(
peer_relationship.cardinality == "many" and peer_relationship.peer == node_schema.kind
for peer_relationship in node_schema_map[relationship.peer].relationships
)
if has_many_peer:
many_relationship_identifiers[node_schema.kind, relationship.peer] = relationship.identifier
Fix with Cubic

Comment on lines +52 to +54
elif isinstance(data, RelatedNodeBase):
# Handle when value is already a RelatedNode - extract its identifying data
data = {"id": data.id, "hfid": data.hfid, "__typename": data.typename}
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Preserve kind when reusing an existing RelatedNode; otherwise HFID-backed references lose identifying data.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/node/related_node.py, line 52:

<comment>Preserve `kind` when reusing an existing RelatedNode; otherwise HFID-backed references lose identifying data.</comment>

<file context>
@@ -49,6 +49,10 @@ def __init__(self, branch: str, schema: RelationshipSchemaAPI, data: Any | dict,
                 setattr(self, prop, None)
             self._relationship_metadata = None
 
+        elif isinstance(data, RelatedNodeBase):
+            # Handle when value is already a RelatedNode - extract its identifying data
+            data = {"id": data.id, "hfid": data.hfid, "__typename": data.typename}
</file context>
Suggested change
elif isinstance(data, RelatedNodeBase):
# Handle when value is already a RelatedNode - extract its identifying data
data = {"id": data.id, "hfid": data.hfid, "__typename": data.typename}
elif isinstance(data, RelatedNodeBase):
# Handle when value is already a RelatedNode - extract its identifying data
data = {"id": data.id, "hfid": data.hfid, "kind": data.kind, "__typename": data.typename}
Fix with Cubic

@@ -17,6 +17,7 @@ $ infrahubctl load [OPTIONS]
* `--branch TEXT`: Branch from which to export
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Don't hand-edit this generated CLI doc; update the source definition and regenerate the MDX instead.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/docs/infrahubctl/infrahubctl-load.mdx, line 20:

<comment>Don't hand-edit this generated CLI doc; update the source definition and regenerate the MDX instead.</comment>

<file context>
@@ -17,6 +17,7 @@ $ infrahubctl load [OPTIONS]
 * `--branch TEXT`: Branch from which to export
 * `--concurrent INTEGER`: Maximum number of requests to execute at the same time.  [env var: INFRAHUB_MAX_CONCURRENT_EXECUTION]
 * `--timeout INTEGER`: Timeout in sec  [env var: INFRAHUB_TIMEOUT; default: 60]
+* `--allow-upsert / --no-allow-upsert`: Use Upsert mutations instead of Create. Use when objects may already exist.  [default: no-allow-upsert]
 * `--install-completion`: Install completion for the current shell.
 * `--show-completion`: Show completion for the current shell, to copy it or customize the installation.
</file context>
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant