Add annotations for clients and client_type #708
Conversation
Also fixes getattr approach to collecting the previously untyped clients variable depending on which client the test needs.
WalkthroughTest function signatures in Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (tests/AGENTS.md)
Files:
tests/unit/**/*.py📄 CodeRabbit inference engine (tests/AGENTS.md)
Files:
🧠 Learnings (5)📚 Learning: 2025-12-10T17:13:08.136ZApplied to files:
📚 Learning: 2025-12-10T17:13:08.136ZApplied to files:
📚 Learning: 2025-12-10T17:13:37.990ZApplied to files:
📚 Learning: 2025-12-10T17:13:21.977ZApplied to files:
📚 Learning: 2025-12-10T17:13:29.593ZApplied to files:
🧬 Code graph analysis (1)tests/unit/sdk/test_node.py (2)
⏰ 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). (1)
🔇 Additional comments (1)
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 |
Deploying infrahub-sdk-python with
|
| Latest commit: |
f3edb52
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://438aa619.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-test-node-annotations.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #708 +/- ##
=================================================
Coverage 76.28% 76.28%
=================================================
Files 114 114
Lines 9830 9830
Branches 1508 1508
=================================================
Hits 7499 7499
Misses 1837 1837
Partials 494 494
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/unit/sdk/test_node.py (2)
1845-1850: Critical: Async/sync mismatch in sync branch.Line 1850 instantiates
tag_blueasInfrahubNode(async) in the sync branch, but it should beInfrahubNodeSyncto match the client type. This violates the guideline to never mix async/sync inappropriately.🔎 Proposed fix
else: location = InfrahubNodeSync(client=client, schema=location_schema, data=location_data) - tag_blue = InfrahubNode(client=client, schema=tag_schema, data=tag_blue_data) + tag_blue = InfrahubNodeSync(client=client, schema=tag_schema, data=tag_blue_data)As per coding guidelines, never mix async/sync inappropriately.
2154-2172: Critical: Async/sync mismatch in sync branch.Line 2168 instantiates
deviceasInfrahubNode(async) in the sync branch, but it should beInfrahubNodeSyncto match the client type. This is inconsistent withip_prefixandip_poolwhich correctly useInfrahubNodeSync.🔎 Proposed fix
else: ip_prefix = InfrahubNodeSync(client=client, schema=ipam_ipprefix_schema, data=ipam_ipprefix_data) ip_pool = InfrahubNodeSync( client=client, schema=ipaddress_pool_schema, data={ "id": "pppppppp-pppp-pppp-pppp-pppppppppppp", "name": "Core loopbacks", "default_address_type": "IpamIPAddress", "default_prefix_length": 32, "ip_namespace": "ip_namespace", "resources": [ip_prefix], }, ) - device = InfrahubNode( + device = InfrahubNodeSync( client=client, schema=simple_device_schema, data={"name": "device-01", "primary_address": ip_pool, "ip_address_pool": ip_pool}, )As per coding guidelines, never mix async/sync inappropriately.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/unit/sdk/test_node.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
tests/unit/sdk/test_node.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.py: Usehttpx_mockfixture for HTTP mocking in tests instead of making real HTTP requests
Do not add@pytest.mark.asynciodecorator to async test functions (async auto-mode is globally enabled)
Files:
tests/unit/sdk/test_node.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
Unit tests must be fast, mocked, and have no external dependencies
Files:
tests/unit/sdk/test_node.py
🧠 Learnings (5)
📚 Learning: 2025-12-10T17:13:08.136Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:08.136Z
Learning: Applies to infrahub_sdk/client.py : Always provide both async and sync versions of client implementations in InfrahubClient
Applied to files:
tests/unit/sdk/test_node.py
📚 Learning: 2025-12-10T17:13:08.136Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:08.136Z
Learning: Applies to infrahub_sdk/**/*.py : Follow async/sync dual pattern for new features in the Python SDK
Applied to files:
tests/unit/sdk/test_node.py
📚 Learning: 2025-12-10T17:13:37.990Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:37.990Z
Learning: Applies to tests/**/*.py : Use `httpx_mock` fixture for HTTP mocking in tests instead of making real HTTP requests
Applied to files:
tests/unit/sdk/test_node.py
📚 Learning: 2025-12-10T17:13:21.977Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/ctl/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:21.977Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Do not instantiate InfrahubClient directly; always use initialize_client() or initialize_client_sync() helper functions
Applied to files:
tests/unit/sdk/test_node.py
📚 Learning: 2025-12-10T17:13:29.593Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/pytest_plugin/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:29.593Z
Learning: Applies to infrahub_sdk/pytest_plugin/**/infrahub_sdk/pytest_plugin/items/*.py : Inherit from `InfrahubItem` base class when creating new test item classes in `infrahub_sdk/pytest_plugin/items/`
Applied to files:
tests/unit/sdk/test_node.py
🧬 Code graph analysis (1)
tests/unit/sdk/test_node.py (2)
infrahub_sdk/node/node.py (6)
InfrahubNode(490-1295)generate_query_data(794-855)generate_query_data(1599-1659)InfrahubNodeSync(1298-2103)generate_query_data_node(857-963)generate_query_data_node(1661-1767)infrahub_sdk/client.py (14)
get(389-406)get(409-426)get(429-446)get(449-466)get(469-486)get(489-506)get(508-568)get(2282-2299)get(2302-2319)get(2322-2339)get(2342-2359)get(2362-2379)get(2382-2399)get(2401-2461)
⏰ 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). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
tests/unit/sdk/test_node.py (1)
135-135: LGTM: Type annotations and explicit client selection improve type safety.The addition of type annotations (
client_type: str,clients: BothClients,httpx_mock: HTTPXMock) aligns with the coding guidelines. The refactor fromgetattr(clients, client_type)to explicitclients.standardorclients.syncimproves type safety and makes the code more maintainable.As per coding guidelines, type hints on all function signatures are required.
Also applies to: 299-307, 399-405, 441-447, 503-509
Also fixes getattr approach to collecting the previously untyped clients variable depending on which client the test needs.
Changes related to #707.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.