Fix missing hfid field on RelatedIPAddressNodeInput#8579
Fix missing hfid field on RelatedIPAddressNodeInput#8579BeArchiTek wants to merge 4 commits intostablefrom
Conversation
RelatedIPAddressNodeInput was missing the hfid field, preventing IP address relationships from being referenced by human-friendly ID. Both RelatedNodeInput and RelatedIPPrefixNodeInput already had this field. Fixes #8574 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe pull request adds an optional 🚥 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 |
Merging this PR will improve performance by 38.69%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_schemabranch_process |
1,026.1 ms | 784.5 ms | +30.8% |
| ⚡ | test_relationshipmanager_getpeer |
155.9 µs | 132.5 µs | +17.68% |
| ⚡ | test_get_menu |
245.4 ms | 188 ms | +30.5% |
| ⚡ | test_query_rel_one |
662.3 ms | 500.5 ms | +32.32% |
| ⚡ | test_schemabranch_duplicate |
7.2 ms | 5.5 ms | +31.49% |
| ⚡ | test_query_rel_many |
693.3 ms | 522.9 ms | +32.61% |
| ⚡ | test_query_one_model |
464.3 ms | 352.8 ms | +31.62% |
| ⚡ | test_get_schema |
323.6 ms | 247.6 ms | +30.72% |
| ⚡ | test_graphql_generate_schema |
516.1 ms | 372.1 ms | +38.69% |
| ⚡ | test_load_node_to_db_node_schema |
66.2 ms | 51.5 ms | +28.64% |
| ⚡ | test_nodemanager_querypeers |
1.5 ms | 1.2 ms | +21.9% |
| ⚡ | test_base_schema_duplicate_CoreProposedChange |
2.1 ms | 1.7 ms | +22.92% |
Comparing fix/8574-ip-address-hfid-input-v2 (4b2e759) with stable (c396f66)1
Footnotes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/infrahub/graphql/types/attribute.py`:
- Line 42: Add a regression test that exercises creating and updating an
IP-address relationship via the new hfid field: write a test that calls the
IpamIPAddress mutation using RelatedIPAddressNodeInput with hfid set (e.g.,
passing an HFID that resolves to an IpamIPAddress and asserting ip_prefix is
set), then also exercise the existing id and from_pool paths to ensure they
still work (create/update via id and via from_pool and assert the same resulting
ip_prefix/relationship). Target test helpers that construct GraphQL inputs for
RelatedIPAddressNodeInput and calls to IpamIPAddress mutation, and include
assertions verifying the relationship was created/updated and no regressions on
id/from_pool behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2353329-cacc-43bd-bd6a-4b6a942bb12f
📒 Files selected for processing (3)
backend/infrahub/graphql/types/attribute.pychangelog/+8574-ip-address-hfid.fixed.mdschema/schema.graphql
Adds a component test that verifies a relationship targeting IpamIPAddress can be referenced by human-friendly ID in a GraphQL create mutation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| "nodes": [ | ||
| { | ||
| "name": "IPPrefix", | ||
| "namespace": "Ipam", | ||
| "default_filter": "prefix__value", | ||
| "order_by": ["prefix__value"], | ||
| "display_labels": ["prefix__value"], | ||
| "branch": "aware", | ||
| "inherit_from": [InfrahubKind.IPPREFIX], | ||
| }, | ||
| { | ||
| "name": "IPAddress", | ||
| "namespace": "Ipam", | ||
| "default_filter": "address__value", | ||
| "order_by": ["address__value"], | ||
| "display_labels": ["address__value"], | ||
| "human_friendly_id": ["address__value"], | ||
| "branch": "aware", | ||
| "inherit_from": [InfrahubKind.IPADDRESS], | ||
| }, |
There was a problem hiding this comment.
I think there's an existing ipam_schema fixture in component/conftest.py that you could copy and update to have an HFID instead of defining the whole schema here, which would reduce the size of the test. also, looks like IpamIPPrefix is not actually used
| assert not result.errors | ||
| assert result.data | ||
| assert result.data["TestDeviceCreate"]["ok"] | ||
| assert result.data["TestDeviceCreate"]["object"]["primary_ip"]["node"]["address"]["value"] == "10.0.0.1/32" |
There was a problem hiding this comment.
it's also good to test retrieving the new object from the database using NodeManager and confirming the data to ensure that the changes make it all the way to the db. in this test it is possible that the changes are just made in the mutation and discarded before actually being saved
Summary by CodeRabbit
New Features
Tests
Documentation / Types
hfidfield toRelatedIPAddressNodeInputGraphQL input type, aligning it withRelatedNodeInputandRelatedIPPrefixNodeInputschema/schema.graphqlto include the new fieldFixes #8574
Test plan
RelatedIPAddressNodeInputnow exposeshfidin the GraphQL schemafrom_poolandidbased IP address references still work🤖 Generated with Claude Code