Skip to content

Conversation

@minitriga
Copy link
Contributor

@minitriga minitriga commented Nov 13, 2025

…dinality one with None data.

Fixes: #630

Summary by CodeRabbit

  • Bug Fixes
    • Fixed handling of optional one-to-one relationships. Uninitialized optional relationships are now properly excluded from mutation requests instead of being set to None.

@coderabbitai
Copy link

coderabbitai bot commented Nov 13, 2025

Walkthrough

This change addresses how optional one-to-one relationships are handled when generating mutation input data. Previously, uninitialized optional relationships were explicitly set to None in the mutation payload. The fix modifies the input data generation to omit these keys entirely instead of including them with null values. Tests have been updated accordingly to reflect these new expectations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main change: preventing InfrahubNode from instantiating optional relationships of cardinality one with None data, which directly matches the changelog and code modifications.
Linked Issues check ✅ Passed The changes directly address issue #630 by preventing the SDK from sending explicit null values for optional one-to-one relationships, allowing the backend to apply object template values as intended.
Out of Scope Changes check ✅ Passed All changes are scoped to the issue: modifications to _generate_input_data method, corresponding test updates, and changelog entry—no extraneous changes detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch atg-20251113-ihs-179

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fd8376 and 4eb8bcb.

📒 Files selected for processing (3)
  • changelog/630.fixed.md (1 hunks)
  • infrahub_sdk/node/node.py (0 hunks)
  • tests/unit/sdk/test_node.py (4 hunks)
💤 Files with no reviewable changes (1)
  • infrahub_sdk/node/node.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: unit-tests (3.13)
  • GitHub Check: unit-tests (3.10)
  • GitHub Check: unit-tests (3.9)
  • GitHub Check: unit-tests (3.11)
  • GitHub Check: unit-tests (3.12)
  • GitHub Check: integration-tests-latest-infrahub
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
tests/unit/sdk/test_node.py (3)

1632-1643: LGTM! Test correctly reflects the new behavior.

The test now expects only the address field in the generated input data, omitting any uninitialized optional relationships. This aligns with the PR objective of not sending null values for optional cardinality-one relationships.


1646-1657: LGTM! Test correctly reflects the new behavior.

The test now expects only the network field in the generated input data, omitting any uninitialized optional relationships. This aligns with the PR objective of not sending null values for optional cardinality-one relationships.


1825-1872: No issues found — behavior is correct and intentional.

When primary_tag is explicitly set to None, the RelatedNode.initialized property correctly returns False (because neither id nor hfid is set). The _generate_input_data() method then skips optional cardinality-one relationships that are not initialized, omitting them from the mutation payload. This prevents sending explicit nulls to the backend and aligns with issue #630.

The different behavior between cardinality-one and cardinality-many relationships is intentional:

  • primary_tag (cardinality-one): Omitted when uninitialized
  • tags (cardinality-many): Returns empty array [] when cleared

This design allows users to distinguish between "not setting a value" (omit the field) and "clearing a value" (send empty array for many-relations), which is correct.

changelog/630.fixed.md (1)

1-1: LGTM! Clear and accurate changelog entry.

The changelog entry succinctly describes the fix and aligns with the PR objectives.


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

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4eb8bcb
Status: ✅  Deploy successful!
Preview URL: https://293bc896.infrahub-sdk-python.pages.dev
Branch Preview URL: https://atg-20251113-ihs-179.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

❗ There is a different number of reports uploaded between BASE (4fd8376) and HEAD (4eb8bcb). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (4fd8376) HEAD (4eb8bcb)
integration-tests 1 0
@@            Coverage Diff             @@
##           stable     #631      +/-   ##
==========================================
- Coverage   75.48%   69.84%   -5.64%     
==========================================
  Files         113      113              
  Lines        9512     9511       -1     
  Branches     1893     1893              
==========================================
- Hits         7180     6643     -537     
- Misses       1832     2365     +533     
- Partials      500      503       +3     
Flag Coverage Δ
integration-tests ?
python-3.10 48.76% <ø> (-0.03%) ⬇️
python-3.11 48.76% <ø> (-0.03%) ⬇️
python-3.12 48.74% <ø> (-0.01%) ⬇️
python-3.13 48.72% <ø> (-0.05%) ⬇️
python-3.9 47.45% <ø> (-0.03%) ⬇️
python-filler-3.12 24.29% <ø> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
infrahub_sdk/node/node.py 70.83% <ø> (-5.92%) ⬇️

... and 25 files with indirect coverage changes

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

Copy link
Contributor

@ogenstad ogenstad left a comment

Choose a reason for hiding this comment

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

We need to consider this a bit more, it looks like we're reverting the things done in #515 with this PR. So we need to take a closer look at that's going on here.

@minitriga
Copy link
Contributor Author

Closing for now as I have a work around.

@minitriga minitriga closed this Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Creating an object with an object template with optional attributes fails to set the optional attributes

3 participants