-
Notifications
You must be signed in to change notification settings - Fork 6
Merge stable into develop with resolved conflicts #506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* document INFRAHUB_PROXY * vale * corrections * fix tabs * rabbit * sanity checking * Complete rework of client guide * Lint * Vale --------- Co-authored-by: Baptiste <[email protected]>
Auto fix return type None for tests
WalkthroughThis pull request updates documentation, linting configuration, and tests:
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying infrahub-sdk-python with
|
| Latest commit: |
b24a04d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://71379038.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-stable-to-develop-202508.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## develop #506 +/- ##
========================================
Coverage 75.74% 75.74%
========================================
Files 100 100
Lines 8846 8846
Branches 1732 1732
========================================
Hits 6700 6700
Misses 1670 1670
Partials 476 476
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/unit/sdk/test_group_context.py (1)
68-82: Ensure pytest-asyncio is configured to run async fixtures in “auto” modeWe’ve confirmed that the repo includes pytest-asyncio (<0.23) as a dev dependency but does not set
asyncio_modeanywhere (no pytest.ini, pyproject.toml, tox.ini, or setup.cfg entry). In pytest-asyncio >= 0.19 and < 0.23 the defaultasyncio_modeis strict, which means:
- Async fixtures without
@pytest_asyncio.fixture(likestd_group_schema) won’t be driven by the pytest-asyncio plugin.- Your sync-style test calling
std_group_schemamay silently skip or error out under CI or on fresh environments.Please add one of the following to ensure your async fixtures run as intended:
Create or update
pytest.iniin your repo root:[pytest] asyncio_mode = autoOR decorate your async fixtures explicitly:
import pytest_asyncio @pytest_asyncio.fixture async def std_group_schema(): …This will guarantee that
std_group_schemais executed by pytest-asyncio rather than left unresolved.tests/unit/sdk/test_utils.py (1)
129-131: This raises TypeError before calling str_to_bool; fix the test to exercise the function
tuple("a", "b", "c")raises TypeError at tuple construction (too many args). Use a tuple value to letstr_to_boolraise the error.- with pytest.raises(TypeError): - str_to_bool(tuple("a", "b", "c")) + with pytest.raises(TypeError): + str_to_bool(("a", "b", "c"))tests/unit/sdk/test_schema.py (1)
191-202: Fix mismatch: test name says “remove” but sync branch calls “add”In
test_remove_enum_option_raises, the sync branch usesadd_enum_optioninstead ofremove_enum_option, reducing coverage of the remove path. Update to call the correct method.Apply this diff:
else: - with pytest.raises(SchemaNotFoundError): - clients.sync.schema.add_enum_option("DoesNotExist", "atribute", "option") - with pytest.raises(ValueError): - clients.sync.schema.add_enum_option("BuiltinTag", "attribute", "option") + with pytest.raises(SchemaNotFoundError): + clients.sync.schema.remove_enum_option("DoesNotExist", "atribute", "option") + with pytest.raises(ValueError): + clients.sync.schema.remove_enum_option("BuiltinTag", "attribute", "option")tests/unit/sdk/test_node.py (1)
1849-1854: The__getitem__implementation simply returnsself.peers[item], and sinceself.peersis just a standard Python list, indexing an out-of-range index will indeed raise anIndexError, notNodeNotFoundError. Thus your testwith pytest.raises(NodeNotFoundError): _ = node.tags[0].peerwill fail with
IndexErrorwhennode.tagsis empty.To ensure the test consistently raises
NodeNotFoundError, you have two options:• Use a single-relationship accessor (e.g.
node.primary_tag.peer) which already checks “not in store” and raisesNodeNotFoundError.
• Prepopulatenode.tagswith an ID-only related node (so__getitem__succeeds) and then call.peer, triggering theNodeNotFoundErrorpath.Please update the test accordingly.
🧹 Nitpick comments (41)
tests/unit/sdk/test_yaml.py (1)
51-55: Explicit -> None annotation looks good; consider using exc.match for cleaner assertionThe change is fine. As a minor improvement, pytest’s exc.match reads cleaner than converting the exception to string.
Apply this diff:
-def test_load_non_existing_folder() -> None: - with pytest.raises(FileNotValidError) as exc: - YamlFile.load_from_disk(paths=[get_fixtures_dir() / "does_not_exist"]) - assert "does not exist" in str(exc.value) +def test_load_non_existing_folder() -> None: + with pytest.raises(FileNotValidError) as excinfo: + YamlFile.load_from_disk(paths=[get_fixtures_dir() / "does_not_exist"]) + excinfo.match("does not exist")tests/integration/test_schema.py (1)
22-22: Consider relaxing the exact error string comparison to reduce brittleness.Exact message text can drift across versions. Matching a stable substring keeps intent while easing maintenance, e.g.:
with pytest.raises(BranchNotFoundError) as exc: await client.all(kind="BuiltinTag", branch="I-do-not-exist") assert "branch was not found" in str(exc.value)tests/unit/sdk/test_timestamp.py (3)
12-20: Prefer public API over private attribute in assertions.Accessing
_objdirectly is fragile. You can assert equivalence via the public getter to reduce coupling to internals:- assert t1.to_datetime() == t1._obj.py_datetime() + assert t1.to_datetime() == t1.get_obj().py_datetime() ... - assert t2.to_datetime() == t2._obj.py_datetime() + assert t2.to_datetime() == t2.get_obj().py_datetime()
30-43: Relative-time parsing assertions are non-deterministic; add a type or tz assertion for stability.Asserting truthiness is fine, but you can strengthen these without freezing time by checking the result type or UTC normalization:
- Option A: assert type
assert Timestamp._parse_string("5m").__class__.__name__ == "ZonedDateTime"(or import the type)- Option B: assert UTC round-trip
assert Timestamp("5m").to_string().endswith("Z")Keeps the tests stable while remaining time-agnostic.
66-69: Minor naming nit: id label is misleading.The case labeled
"milliseconds_with_offset"uses aZsuffix (no offset) and a 6-digit fraction (microseconds). Consider renaming the id to reflect microseconds and no offset for clarity.tests/unit/sdk/test_protocols_generator.py (1)
39-41: Explicit return type on async test: good; remove redundant assertThe added -> None is clear. There's a duplicate assertion that can be removed without losing coverage.
async def test_filter_syncify(test_case: SyncifyTestCase) -> None: - assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.output assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.output.vale/styles/Infrahub/sentence-case.yml (2)
42-42: Quote the entry containing '&' to avoid YAML edge casesEven though this parses as a plain scalar, quoting avoids accidental anchor interpretation and improves readability.
- - include_in_menu & menu_placement + - "include_in_menu & menu_placement"
28-29: Consider alphabetizing the exceptions list to reduce future merge conflictsThe list appears semi-ordered; keeping it strictly alphabetical makes reviews easier and prevents duplicates from creeping in over time.
Also applies to: 46-49
tests/unit/sdk/test_branch.py (2)
24-31: Type the parameter for claritySince async_branch_methods is built from dir(...), items are str. Annotate the parameter for better type-checking.
- def test_validate_method_signature(method) -> None: + def test_validate_method_signature(method: str) -> None:
34-42: Async test annotation: LGTM; optional fixture typingThe -> None is fine. Optionally, annotate fixture types to help static analysis (imports required: BothClients, HTTPXMock).
-async def test_get_branches(clients, mock_branches_list_query, client_type) -> None: +async def test_get_branches( + clients: BothClients, # from tests.unit.sdk.conftest + mock_branches_list_query: HTTPXMock, # from pytest_httpx + client_type: str, +) -> None:tests/unit/sdk/test_object_store.py (1)
46-54: Parametrization could include non-callables; consider filtering to callables for robustnessdir(...) may include non-callable attributes; inspect.signature on them would fail. Tests likely pass today, but this hardens against future additions.
Outside the changed lines, consider generating method lists like:
import inspect async_methods = [ name for name, attr in inspect.getmembers(ObjectStore) if callable(attr) and not name.startswith("_") ] sync_methods = [ name for name, attr in inspect.getmembers(ObjectStoreSync) if callable(attr) and not name.startswith("_") ]tests/unit/ctl/test_schema_app.py (1)
84-131: Verify regex pattern string in expected error messageThe mocked API response’s first error includes pattern '^[A-Z][a-z0-9]+$', while expected_result asserts "String should match pattern '^[A-Z]+$'". If the CLI intentionally normalizes/simplifies the pattern in its user-facing output, all good; otherwise this may be brittle if server/CLI messaging diverges.
Action options:
- Confirm the CLI intentionally rewrites the pattern and leave as-is.
- Or update expected_result (or derive the pattern from the mocked response) to reduce duplication/mismatch risk.
tests/unit/sdk/spec/test_object.py (1)
112-125: Avoid shadowing built-in name "format" in parametrized test.Using "format" as a parameter shadows the Python built-in. Not harmful here but easy to avoid; suggest renaming to data_format for clarity.
Apply this diff within the selected block:
-@pytest.mark.parametrize("data,is_valid,format", get_relationship_info_testdata) +@pytest.mark.parametrize("data,is_valid,data_format", get_relationship_info_testdata) async def test_get_relationship_info_tags( client: InfrahubClient, mock_schema_query_01: HTTPXMock, data: dict | list, is_valid: bool, - format: RelationshipDataFormat, + data_format: RelationshipDataFormat, ) -> None: @@ - assert rel_info.format == format + assert rel_info.format == data_formatCLAUDE.md (2)
91-96: Tighten wording for the async/sync pattern.Minor grammar tweak for readability.
-All operations follow dual implementation pattern: +All operations follow a dual-implementation pattern:
107-110: Clarify proxy configuration mutual exclusivity.Make it explicit that the single-proxy and per-scheme mounts cannot be used together.
-- Proxy configuration: `INFRAHUB_PROXY` (single) or `INFRAHUB_PROXY_MOUNTS_HTTP`/`INFRAHUB_PROXY_MOUNTS_HTTPS` (separate) -- Mutual exclusivity validation between proxy configuration methods +- Proxy configuration: use either `INFRAHUB_PROXY` (single proxy for all schemes) or the pair + `INFRAHUB_PROXY_MOUNTS_HTTP` / `INFRAHUB_PROXY_MOUNTS_HTTPS` (separate proxies). +- These options are mutually exclusive; setting both should raise a configuration error.tests/unit/sdk/test_group_context.py (2)
7-8: Filter only callables when enumerating public methods.
Using dir(...) may include non-callable public attributes; inspect.signature() will then fail. Consider filtering by callable() to harden the test.-async_methods = [method for method in dir(InfrahubGroupContext) if not method.startswith("_")] -sync_methods = [method for method in dir(InfrahubGroupContextSync) if not method.startswith("_")] +async_methods = [ + name for name, member in inspect.getmembers(InfrahubGroupContext) + if not name.startswith("_") and callable(member) +] +sync_methods = [ + name for name, member in inspect.getmembers(InfrahubGroupContextSync) + if not name.startswith("_") and callable(member) +]
10-11: Remove unused variable.
client_types is declared but not used in this file. Safe to delete to avoid noise.-client_types = ["standard", "sync"] -tests/unit/ctl/test_cli.py (1)
29-33: LGTM on annotation; optional enhancement.
You may assert a version pattern via regex for extra confidence, but current checks are acceptable.tests/unit/sdk/test_topological_sort.py (1)
76-95: Annotation accepted; minor type-hint consistency nit.
Tests use int keys in some cases while implementation hints Mapping[str, Iterable[str]]. Functionally fine, but consider using str keys across tests for type consistency.tests/unit/ctl/test_validate_app.py (1)
36-42: Normalize stdout before matching error text.
Other tests strip ANSI codes; do the same here for consistency and to avoid flaky failures when colors are enabled.result = runner.invoke(app=app, args=["schema", str(fixture_file)]) - assert result.exit_code == 1 - assert "Invalid JSON file" in result.stdout + assert result.exit_code == 1 + assert "Invalid JSON file" in remove_ansi_color(result.stdout)Also, since new assertions clarify the expectation, consider removing xfail once the underlying exception handling is fixed and the test passes consistently.
docs/docs/python-sdk/guides/client.mdx (5)
340-362: Add Sync variant for “Enable debug logging”To satisfy “include both async and sync examples,” show InfrahubClientSync usage as well. Also pluralize “variables.”
- ```python - from infrahub_sdk import Config, InfrahubClient - config = Config(echo_graphql_queries=True) - client = InfrahubClient(config=config) - ``` + ```python + from infrahub_sdk import Config, InfrahubClient, InfrahubClientSync + # Async + config = Config(echo_graphql_queries=True) + client = InfrahubClient(config=config) + # Sync + client_sync = InfrahubClientSync(config=config) + ``` @@ - client = InfrahubClient() # debug flag state is read from the INFRAHUB_ECHO_GRAPHQL_QUERIES environment variables + client = InfrahubClient() # debug flag state is read from the INFRAHUB_ECHO_GRAPHQL_QUERIES environment variable + # Or Sync: + # from infrahub_sdk import InfrahubClientSync + # client = InfrahubClientSync()
95-100: Tighten wording: “Export … as environment variables” (plural) and remove “We need to …”Minor phrasing improvements. Also fix singular “variable” to “variables.”
-We need to export the address and API token as an environment variable: +Export the address and API token as environment variables: @@ -We need to export the address and API token as an environment variable: +Export the address and API token as environment variables: @@ -We need to export the address, username and password as environment variables: +Export the address, username and password as environment variables: @@ -We need to export the address, username and password as environment variables: +Export the address, username and password as environment variables:Also applies to: 126-131, 162-166, 193-196
74-74: Unify link style to the Configuration referenceThe link at Line 74 uses “config.mdx” while the “Related resources” section links to “../reference/config”. For Docusaurus, prefer extensionless links consistently.
-You can find the full list of configuration options in the [SDK configuration reference](../reference/config.mdx). +You can find the full list of configuration options in the [SDK configuration reference](../reference/config).Also applies to: 446-447
268-312: Prefer context managers to close clients in examplesTo avoid lingering connections in tutorials, show
async with/context-manager patterns (you already used it in Step 3 after the suggested edits).- client = InfrahubClient(config=Config(address="http://localhost:8000")) + async with InfrahubClient(config=Config(address="http://localhost:8000")) as client: @@ - client = InfrahubClientSync(config=Config(address="http://localhost:8000")) + with InfrahubClientSync(config=Config(address="http://localhost:8000")) as client:
371-399: Add aConfig(proxy_mounts=…)example under “Separate proxies for HTTP and HTTPS”We’ve confirmed that:
- The SDK’s
Configmodel definesproxy_mounts: ProxyMountsConfigwithhttp/httpsfields (backed by env varsINFRAHUB_PROXY_MOUNTS_HTTPandINFRAHUB_PROXY_MOUNTS_HTTPS).- Passing both
proxyandproxy_mountstoConfig()triggers theValueError("'proxy' and 'proxy_mounts' are mutually exclusive")validator.- Pydantic’s BaseSettings semantics ensure that any explicit parameters (e.g.
proxy_mounts={...}) override environment-derived values.Please insert the following snippet immediately beneath the “Separate proxies for HTTP and HTTPS” section in
docs/docs/python-sdk/guides/client.mdx(around line 403):In some network environments, you might need to route HTTP and HTTPS traffic through different proxy servers. Use the `INFRAHUB_PROXY_MOUNTS_HTTP` and `INFRAHUB_PROXY_MOUNTS_HTTPS` environment variables for this purpose: - export INFRAHUB_PROXY_MOUNTS_HTTP=http://http-proxy.example.com:8080 - export INFRAHUB_PROXY_MOUNTS_HTTPS=http://https-proxy.example.com:8080 + # Or configure via the Config object: + ```python + from infrahub_sdk import Config, InfrahubClient, InfrahubClientSync + + config = Config(proxy_mounts={ + "http": "http://http-proxy.example.com:8080", + "https": "http://https-proxy.example.com:8080", + }) + client = InfrahubClient(config=config) + client_sync = InfrahubClientSync(config=config) + ```This will clearly demonstrate how to use the
ConfigAPI for split proxies alongside the existing environment-variable examples.tests/integration/test_spec_object.py (4)
56-58: Add an assertion to verify the branch is actually createdThis test currently exercises the create path but doesn’t assert any outcome.
- await client.branch.create(branch_name=branch_name, sync_with_git=False) + branch = await client.branch.create(branch_name=branch_name, sync_with_git=False) + assert getattr(branch, "id", None) is not None
74-76: Comment contradicts the assertionThe comment says “not present” but you assert there are 3 items. Adjust the wording.
- # Check that the nodes are not present in the database before loading the file - assert len(await client.all(kind=obj_file.spec.kind, branch=branch_name)) == 3 + # Confirm expected pre-state before loading the file (3 existing tags) + assert len(await client.all(kind=obj_file.spec.kind, branch=branch_name)) == 3
133-137: Typo: “Parler” → “Parker”Minor typo in the comment.
- # Validate that animals for Emily Parler have been correctly created + # Validate that animals for Emily Parker have been correctly created
144-151: Avoid variable shadowing and improve readability
menuis reused as both the list and the loop variable in the dict comprehension, which is easy to misread.- menu = await client.filters(kind=menu_file.spec.kind, protected__value=False, branch=branch_name) - assert len(menu) == 3 - - menu_by_name = {menu.display_label: menu for menu in menu} - await menu_by_name["Animals"].children.fetch() - peer_labels = [peer.display_label for peer in menu_by_name["Animals"].children.peers] + items = await client.filters(kind=menu_file.spec.kind, protected__value=False, branch=branch_name) + assert len(items) == 3 + + menu_by_name = {m.display_label: m for m in items} + await menu_by_name["Animals"].children.fetch() + peer_labels = [peer.display_label for peer in menu_by_name["Animals"].children.peers]tests/unit/sdk/test_utils.py (2)
46-61: Rename parametrize fields to avoid shadowing builtins and add parameter typesUsing the name “input” shadows Python’s built-in. Rename for clarity and keep the parametrize arguments in sync.
-@pytest.mark.parametrize( - "input,result", +@pytest.mark.parametrize( + "url_input,expected", @@ -) -def test_is_valid_url(input, result) -> None: - assert is_valid_url(input) is result +) +def test_is_valid_url(url_input: object, expected: bool) -> None: + assert is_valid_url(url_input) is expected
198-213: Ensure temp directory cleanup even on assertion failuresUse a context manager to guarantee cleanup if a prior assertion fails.
-def test_write_to_file() -> None: - tmp_dir = tempfile.TemporaryDirectory() - directory = Path(tmp_dir.name) +def test_write_to_file() -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + directory = Path(tmp_dir) @@ - tmp_dir.cleanup() + # cleanup handled by the context managertests/unit/sdk/test_graphql.py (1)
118-159: Optional: reduce duplication in indentation/offset assertions via parametrizationThe two blocks that assert identical structures with different indent/offset are good, but could be parametrized to avoid duplication and ease future updates.
tests/integration/test_infrahub_client.py (2)
36-41: Annotate generator fixture return type for completenessThis is a yield-based async fixture; consider annotating it as an async iterator to align with the rest of the explicit typing work.
Apply this diff:
-from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, AsyncIterator @@ - async def set_pagination_size3(self, client: InfrahubClient): + async def set_pagination_size3(self, client: InfrahubClient) -> AsyncIterator[None]: original_pagination_size = client.pagination_size client.pagination_size = 3 yield client.pagination_size = original_pagination_size
157-160: Nit: tighten regex to match the exact error message punctuationEscaping the trailing period avoids a “match any char” and makes the intent explicit.
Apply this diff:
- with pytest.raises(URLNotFoundError, match=r"/graphql/unexisting` not found."): + with pytest.raises(URLNotFoundError, match=r"/graphql/unexisting` not found\."): await client.execute_graphql(query="unused", branch_name="unexisting")tests/unit/sdk/test_schema.py (1)
205-205: Correct type annotation ofclient_type(it’s a single str from parametrize, not list[str])These tests parametrize
client_typeover ["standard", "sync"], so the variable is astr, not alist[str]. Adjust to avoid misleading annotations.Apply this diff:
-async def test_schema_wait_happy_path(clients: BothClients, client_type: list[str], httpx_mock: HTTPXMock) -> None: +async def test_schema_wait_happy_path(clients: BothClients, client_type: str, httpx_mock: HTTPXMock) -> None: @@ -async def test_schema_set_cache_dict(clients: BothClients, client_type: list[str], schema_query_01_data: dict) -> None: +async def test_schema_set_cache_dict(clients: BothClients, client_type: str, schema_query_01_data: dict) -> None: @@ -async def test_schema_set_cache_branch_schema( - clients: BothClients, client_type: list[str], schema_query_01_data: dict -) -> None: +async def test_schema_set_cache_branch_schema( + clients: BothClients, client_type: str, schema_query_01_data: dict +) -> None:Also applies to: 226-226, 239-241
tests/unit/sdk/test_node.py (6)
69-72: Fix misleading docstring for UNSAFE values testDocstring says values are safe, but this parametrization is for UNSAFE values.
Apply:
-def test_identify_unsafe_graphql_value(value: str) -> None: - """All these values are safe and should not be converted""" +def test_identify_unsafe_graphql_value(value: str) -> None: + """These values are unsafe and must not match SAFE_VALUE."""
88-88: Typo in comment: “comparaison” → “comparison”Minor readability nit.
- # Extract names of parameters and exclude some from the comparaison like client + # Extract parameter names and exclude "client" from the comparison.
1780-1784: Inconsistent node classes in sync branch (use InfrahubNodeSync for tag_blue)Else branch initializes
locationas sync buttag_blueas async. For consistency and to avoid subtle type mixing, prefer the sync class here as well. Functional impact is likely low, but consistency helps.- location = InfrahubNodeSync(client=client, schema=location_schema, data=location_data) - tag_blue = InfrahubNode(client=client, schema=tag_schema, data=tag_blue_data) + location = InfrahubNodeSync(client=client, schema=location_schema, data=location_data) + tag_blue = InfrahubNodeSync(client=client, schema=tag_schema, data=tag_blue_data)
2088-2092: Prefer InfrahubNodeSync for device creation in sync branchThe sync code path still constructs
deviceusing the asyncInfrahubNode. Aligning toInfrahubNodeSyncimproves clarity and avoids accidental async-only behavior creeping into sync tests.- device = InfrahubNode( + device = InfrahubNodeSync( client=client, schema=simple_device_schema, data={"name": "device-01", "primary_address": ip_pool, "ip_address_pool": ip_pool}, )(Apply in both highlighted locations.)
Also applies to: 2141-2144
1535-1536: Track TODO/issue references with xfail/skip when appropriateTests marked with TODO and a tracking issue are fine while passing; if they’re flaky or expected to fail pending issue 214, consider
pytest.mark.xfail(reason="...")or a targetedskipifto encode the expectation.Also applies to: 1730-1731
56-56: Optional: Enforce test function return-type annotations via CI lintAll test functions in
tests/unit/sdk/test_node.pyare already annotated with-> None, so there are no missing annotations to address at this time. Consider adding a CI check (for example, a mypy rule or a simplegrep) to flag any future test functions that lack an explicit-> Nonereturn type.
- File to scope the lint:
tests/unit/sdk/test_node.py
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
docs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
.vale/styles/Infrahub/sentence-case.yml(3 hunks)CLAUDE.md(1 hunks)docs/docs/python-sdk/guides/client.mdx(1 hunks)docs/package.json(2 hunks)tests/conftest.py(1 hunks)tests/integration/test_infrahub_client.py(9 hunks)tests/integration/test_infrahubctl.py(1 hunks)tests/integration/test_node.py(6 hunks)tests/integration/test_repository.py(1 hunks)tests/integration/test_schema.py(1 hunks)tests/integration/test_spec_object.py(7 hunks)tests/unit/ctl/test_branch_app.py(2 hunks)tests/unit/ctl/test_cli.py(2 hunks)tests/unit/ctl/test_schema_app.py(3 hunks)tests/unit/ctl/test_validate_app.py(2 hunks)tests/unit/pytest_plugin/test_plugin.py(6 hunks)tests/unit/sdk/checks/test_checks.py(2 hunks)tests/unit/sdk/spec/test_object.py(4 hunks)tests/unit/sdk/test_artifact.py(6 hunks)tests/unit/sdk/test_batch.py(3 hunks)tests/unit/sdk/test_branch.py(2 hunks)tests/unit/sdk/test_client.py(26 hunks)tests/unit/sdk/test_config.py(1 hunks)tests/unit/sdk/test_diff_summary.py(1 hunks)tests/unit/sdk/test_graphql.py(13 hunks)tests/unit/sdk/test_group_context.py(5 hunks)tests/unit/sdk/test_node.py(50 hunks)tests/unit/sdk/test_object_store.py(3 hunks)tests/unit/sdk/test_protocols_generator.py(1 hunks)tests/unit/sdk/test_query_analyzer.py(6 hunks)tests/unit/sdk/test_repository.py(4 hunks)tests/unit/sdk/test_schema.py(12 hunks)tests/unit/sdk/test_schema_sorter.py(1 hunks)tests/unit/sdk/test_store.py(4 hunks)tests/unit/sdk/test_store_branch.py(4 hunks)tests/unit/sdk/test_task.py(8 hunks)tests/unit/sdk/test_timestamp.py(6 hunks)tests/unit/sdk/test_topological_sort.py(6 hunks)tests/unit/sdk/test_utils.py(8 hunks)tests/unit/sdk/test_uuidt.py(2 hunks)tests/unit/sdk/test_yaml.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
When implementing Infrahub checks, subclass InfrahubCheck and override validate(data); do not implement or rely on a check() method
Files:
tests/unit/sdk/test_store_branch.pytests/unit/sdk/test_yaml.pytests/unit/sdk/test_diff_summary.pytests/conftest.pytests/unit/sdk/test_protocols_generator.pytests/unit/sdk/test_uuidt.pytests/unit/sdk/test_store.pytests/integration/test_repository.pytests/unit/sdk/test_schema_sorter.pytests/unit/sdk/test_config.pytests/unit/pytest_plugin/test_plugin.pytests/unit/ctl/test_schema_app.pytests/unit/ctl/test_validate_app.pytests/integration/test_schema.pytests/unit/sdk/checks/test_checks.pytests/integration/test_infrahubctl.pytests/unit/sdk/test_group_context.pytests/unit/sdk/test_batch.pytests/unit/sdk/test_repository.pytests/integration/test_node.pytests/integration/test_infrahub_client.pytests/unit/sdk/test_object_store.pytests/integration/test_spec_object.pytests/unit/sdk/test_branch.pytests/unit/ctl/test_cli.pytests/unit/sdk/spec/test_object.pytests/unit/ctl/test_branch_app.pytests/unit/sdk/test_graphql.pytests/unit/sdk/test_topological_sort.pytests/unit/sdk/test_task.pytests/unit/sdk/test_timestamp.pytests/unit/sdk/test_utils.pytests/unit/sdk/test_query_analyzer.pytests/unit/sdk/test_client.pytests/unit/sdk/test_node.pytests/unit/sdk/test_artifact.pytests/unit/sdk/test_schema.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write unit tests under tests/unit/ (isolated component tests)
Files:
tests/unit/sdk/test_store_branch.pytests/unit/sdk/test_yaml.pytests/unit/sdk/test_diff_summary.pytests/unit/sdk/test_protocols_generator.pytests/unit/sdk/test_uuidt.pytests/unit/sdk/test_store.pytests/unit/sdk/test_schema_sorter.pytests/unit/sdk/test_config.pytests/unit/pytest_plugin/test_plugin.pytests/unit/ctl/test_schema_app.pytests/unit/ctl/test_validate_app.pytests/unit/sdk/checks/test_checks.pytests/unit/sdk/test_group_context.pytests/unit/sdk/test_batch.pytests/unit/sdk/test_repository.pytests/unit/sdk/test_object_store.pytests/unit/sdk/test_branch.pytests/unit/ctl/test_cli.pytests/unit/sdk/spec/test_object.pytests/unit/ctl/test_branch_app.pytests/unit/sdk/test_graphql.pytests/unit/sdk/test_topological_sort.pytests/unit/sdk/test_task.pytests/unit/sdk/test_timestamp.pytests/unit/sdk/test_utils.pytests/unit/sdk/test_query_analyzer.pytests/unit/sdk/test_client.pytests/unit/sdk/test_node.pytests/unit/sdk/test_artifact.pytests/unit/sdk/test_schema.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use the custom pytest plugin (infrahub_sdk.pytest_plugin) fixtures for clients, configuration, and Infrahub-specific testing
Files:
tests/unit/sdk/test_store_branch.pytests/unit/sdk/test_yaml.pytests/unit/sdk/test_diff_summary.pytests/conftest.pytests/unit/sdk/test_protocols_generator.pytests/unit/sdk/test_uuidt.pytests/unit/sdk/test_store.pytests/integration/test_repository.pytests/unit/sdk/test_schema_sorter.pytests/unit/sdk/test_config.pytests/unit/pytest_plugin/test_plugin.pytests/unit/ctl/test_schema_app.pytests/unit/ctl/test_validate_app.pytests/integration/test_schema.pytests/unit/sdk/checks/test_checks.pytests/integration/test_infrahubctl.pytests/unit/sdk/test_group_context.pytests/unit/sdk/test_batch.pytests/unit/sdk/test_repository.pytests/integration/test_node.pytests/integration/test_infrahub_client.pytests/unit/sdk/test_object_store.pytests/integration/test_spec_object.pytests/unit/sdk/test_branch.pytests/unit/ctl/test_cli.pytests/unit/sdk/spec/test_object.pytests/unit/ctl/test_branch_app.pytests/unit/sdk/test_graphql.pytests/unit/sdk/test_topological_sort.pytests/unit/sdk/test_task.pytests/unit/sdk/test_timestamp.pytests/unit/sdk/test_utils.pytests/unit/sdk/test_query_analyzer.pytests/unit/sdk/test_client.pytests/unit/sdk/test_node.pytests/unit/sdk/test_artifact.pytests/unit/sdk/test_schema.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write integration tests under tests/integration/ (tests against real Infrahub instances)
Files:
tests/integration/test_repository.pytests/integration/test_schema.pytests/integration/test_infrahubctl.pytests/integration/test_node.pytests/integration/test_infrahub_client.pytests/integration/test_spec_object.py
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.{md,mdx}: Follow the Diataxis framework and classify docs as Tutorials, How-to guides, Explanation, or Reference
Structure How-to guides with required frontmatter and sections: introduction, prerequisites, step-by-step steps, validation, related resources
Structure Topics (Explanation) docs with introduction, concepts & definitions, background & context, architecture & design, connections, further reading
Use a professional, concise, informative tone with consistent structure across documents
Use proper language tags on all code blocks
Include both async and sync examples where applicable using the Tabs component
Add validation steps to guides to confirm success and expected outputs
Use callouts for warnings, tips, and important notes
Define new terms on first use and use domain-relevant terminology consistent with Infrahub’s model/UI
Conform to markdown style rules in .markdownlint.yaml and Vale styles in .vale/styles/
Files:
docs/docs/python-sdk/guides/client.mdx
🧬 Code graph analysis (28)
tests/unit/sdk/test_store_branch.py (2)
tests/unit/sdk/test_store.py (4)
test_node_store_set(11-32)test_node_store_set_no_hfid(36-66)test_node_store_get(70-110)test_node_store_get_with_hfid(114-148)tests/unit/sdk/conftest.py (2)
schema_with_hfid(176-237)location_schema(140-172)
tests/unit/sdk/test_diff_summary.py (1)
tests/unit/sdk/conftest.py (2)
clients(37-42)BothClients(25-28)
tests/unit/sdk/test_protocols_generator.py (1)
tests/unit/sdk/conftest.py (2)
client(32-33)mock_schema_query_05(1846-1853)
tests/unit/sdk/test_store.py (1)
tests/unit/sdk/conftest.py (3)
clients(37-42)schema_with_hfid(176-237)location_schema(140-172)
tests/unit/sdk/test_schema_sorter.py (1)
tests/unit/sdk/conftest.py (1)
mock_schema_query_01(1824-1831)
tests/unit/sdk/test_config.py (2)
infrahub_sdk/exceptions.py (1)
ValidationError(115-127)infrahub_sdk/config.py (2)
Config(137-177)password_authentication(133-134)
tests/unit/ctl/test_schema_app.py (1)
infrahub_sdk/ctl/utils.py (1)
get_fixtures_dir(178-181)
tests/unit/ctl/test_validate_app.py (2)
tests/helpers/fixtures.py (1)
get_fixtures_dir(4-7)tests/helpers/cli.py (1)
remove_ansi_color(4-6)
tests/integration/test_schema.py (1)
tests/unit/sdk/conftest.py (1)
client(32-33)
tests/unit/sdk/test_group_context.py (1)
tests/unit/sdk/conftest.py (3)
replace_sync_return_annotation(115-122)replace_async_return_annotation(91-97)std_group_schema(241-251)
tests/unit/sdk/test_batch.py (1)
tests/unit/sdk/conftest.py (2)
clients(37-42)BothClients(25-28)
tests/unit/sdk/test_repository.py (1)
infrahub_sdk/repository.py (2)
GitRepoManager(9-33)active_branch(31-33)
tests/integration/test_infrahub_client.py (3)
tests/integration/test_infrahubctl.py (1)
base_dataset(33-46)infrahub_sdk/branch.py (7)
create(78-85)create(88-95)create(97-131)create(250-257)create(260-267)create(269-302)BranchData(17-25)infrahub_sdk/exceptions.py (1)
URLNotFoundError(146-149)
tests/unit/sdk/test_object_store.py (2)
tests/unit/sdk/test_branch.py (2)
test_method_sanity(17-20)test_validate_method_signature(24-30)tests/unit/sdk/test_client.py (2)
test_method_sanity(32-35)test_validate_method_signature(39-58)
tests/integration/test_spec_object.py (3)
tests/integration/test_infrahub_client.py (1)
test_create_branch(140-143)tests/unit/sdk/conftest.py (1)
client(32-33)tests/integration/test_node.py (1)
initial_schema(13-19)
tests/unit/sdk/test_branch.py (2)
tests/unit/sdk/test_client.py (2)
test_method_sanity(32-35)test_validate_method_signature(39-58)tests/unit/sdk/conftest.py (2)
clients(37-42)mock_branches_list_query(1442-1474)
tests/unit/ctl/test_cli.py (1)
tests/unit/sdk/conftest.py (2)
mock_query_infrahub_version(2194-2196)mock_query_infrahub_user(2200-2203)
tests/unit/sdk/spec/test_object.py (1)
tests/unit/sdk/conftest.py (2)
client(32-33)mock_schema_query_01(1824-1831)
tests/unit/ctl/test_branch_app.py (2)
tests/unit/sdk/conftest.py (1)
mock_branches_list_query(1442-1474)tests/unit/ctl/conftest.py (2)
authentication_error_payload(62-73)mock_branch_create_error(77-95)
tests/unit/sdk/test_topological_sort.py (1)
infrahub_sdk/topological_sort.py (1)
topological_sort(20-40)
tests/unit/sdk/test_task.py (2)
tests/unit/sdk/conftest.py (6)
clients(37-42)mock_query_tasks_01(2567-2577)mock_query_tasks_02_main(2581-2590)mock_query_tasks_empty(2594-2603)mock_query_tasks_03(2607-2616)mock_query_tasks_05(2634-2643)tests/unit/sdk/test_client.py (1)
test_method_get_not_found(404-411)
tests/unit/sdk/test_timestamp.py (1)
infrahub_sdk/timestamp.py (5)
Timestamp(21-200)to_string(90-95)to_datetime(100-101)add(142-170)subtract(172-200)
tests/unit/sdk/test_utils.py (2)
infrahub_sdk/utils.py (8)
generate_short_id(30-32)is_valid_url(215-227)duplicates(105-116)base36encode(35-50)base36decode(53-54)base16encode(61-76)base16decode(57-58)dict_hash(239-245)tests/unit/sdk/conftest.py (2)
query_01(2207-2231)query_02(2235-2277)
tests/unit/sdk/test_query_analyzer.py (2)
tests/unit/sdk/conftest.py (8)
query_01(2207-2231)bad_query_01(2403-2424)query_03(2281-2317)query_introspection(2428-2530)query_02(2235-2277)query_04(2321-2336)query_05(2340-2370)query_06(2374-2399)infrahub_sdk/analyzer.py (2)
GraphQLQueryAnalyzer(32-121)nbr_queries(51-52)
tests/unit/sdk/test_client.py (6)
tests/unit/sdk/test_branch.py (1)
test_method_sanity(17-20)tests/unit/sdk/test_group_context.py (1)
test_method_sanity(13-16)tests/unit/sdk/test_node.py (1)
test_method_sanity(56-59)tests/unit/sdk/test_object_store.py (1)
test_method_sanity(40-43)tests/unit/sdk/test_schema.py (1)
test_method_sanity(28-31)tests/unit/sdk/conftest.py (10)
clients(37-42)mock_query_repository_count(1688-1690)mock_query_infrahub_version(2194-2196)mock_query_infrahub_user(2200-2203)mock_query_repository_page1_2(1709-1755)mock_query_repository_page1_1(1612-1649)mock_query_corenode_page1_1(1653-1684)mock_schema_query_01(1824-1831)mock_query_repository_page1_empty(1694-1705)echo_clients(46-58)
tests/unit/sdk/test_node.py (4)
tests/unit/sdk/test_branch.py (1)
test_method_sanity(17-20)tests/unit/sdk/test_client.py (1)
test_method_sanity(32-35)infrahub_sdk/node/parsers.py (1)
parse_human_friendly_id(6-15)tests/unit/sdk/conftest.py (6)
client(32-33)location_schema(140-172)schema_with_hfid(176-237)clients(37-42)mock_schema_query_02(1835-1842)mock_schema_query_01(1824-1831)
tests/unit/sdk/test_artifact.py (1)
tests/unit/sdk/conftest.py (6)
location_schema(140-172)location_data01(375-412)clients(37-42)mock_rest_api_artifact_fetch(1863-1954)device_schema(1184-1230)device_data(1234-1387)
tests/unit/sdk/test_schema.py (3)
tests/unit/sdk/test_client.py (1)
test_method_sanity(32-35)tests/unit/sdk/conftest.py (6)
mock_schema_query_01(1824-1831)rfile_schema(813-849)clients(37-42)mock_query_mutation_schema_dropdown_add(2149-2151)mock_query_mutation_schema_enum_add(2161-2163)mock_query_mutation_schema_enum_remove(2167-2169)infrahub_sdk/schema/__init__.py (6)
remove_dropdown_option(426-431)remove_dropdown_option(650-655)add_enum_option(374-379)add_enum_option(600-605)remove_enum_option(381-386)remove_enum_option(607-612)
🪛 LanguageTool
CLAUDE.md
[uncategorized] ~72-~72: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Nodes load attributes and relationships on demand - Batch Operations: Support for bul...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~91-~91: There might be a mistake here.
Context: ... ### Async/Sync Pattern All operations follow dual implementation pattern: ```python...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...INFRAHUB_PROXY_MOUNTS_HTTPS (separate) - Mutual exclusivity validation between pr...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...s for Infrahub clients and configuration - Support for testing checks, transforms, ...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ... testing checks, transforms, and queries - Integration with infrahub-testcontainers...
(QB_NEW_EN)
[grammar] ~143-~143: There might be a mistake here.
Context: ...ed**: React/Node.js documentation system - Auto-generation: CLI docs and config r...
(QB_NEW_EN)
[grammar] ~144-~144: There might be a mistake here.
Context: ...fig reference generated via invoke tasks - Multi-format: Guides (task-oriented) a...
(QB_NEW_EN)
[grammar] ~161-~161: There might be a mistake here.
Context: ...ehensive linting and formatting (0.11.0) - mypy: Type checking with strict config...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ... Type checking with strict configuration - yamllint: YAML file validation - **mar...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...ion - yamllint: YAML file validation - markdownlint: Documentation consistenc...
(QB_NEW_EN)
[grammar] ~164-~164: There might be a mistake here.
Context: ...arkdownlint**: Documentation consistency - Vale: Documentation style checking ##...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ... Multi-version Python testing (3.9-3.13) 2. Comprehensive linting pipeline 3. Docume...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ...-3.13) 2. Comprehensive linting pipeline 3. Documentation generation and validation ...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ... Documentation generation and validation 4. Integration testing with Infrahub contai...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ...gration testing with Infrahub containers 5. Coverage reporting ## Key Configuration...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...tool configurations (ruff, mypy, pytest) - tasks.py: Invoke task definitions for ...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...sk definitions for development workflows - .github/workflows/ci.yml: Comprehensiv...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...s/ci.yml**: Comprehensive CI/CD pipeline - docs/package.json: Documentation build...
(QB_NEW_EN)
[grammar] ~188-~188: There might be a mistake here.
Context: ....0.0): Configuration and data validation - httpx: Async/sync HTTP client with pro...
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ...sync/sync HTTP client with proxy support - graphql-core: GraphQL query building a...
(QB_NEW_EN)
[grammar] ~190-~190: There might be a mistake here.
Context: ...re**: GraphQL query building and parsing - ujson: Fast JSON serialization ### Op...
(QB_NEW_EN)
[grammar] ~195-~195: There might be a mistake here.
Context: ... CLI functionality (typer, rich, jinja2) - tests extra: Testing framework extensi...
(QB_NEW_EN)
[grammar] ~196-~196: There might be a mistake here.
Context: ...ts extra**: Testing framework extensions - all extra: Complete development enviro...
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ...nted): Step-by-step learning experiences - How-to guides (task-oriented): Problem...
(QB_NEW_EN)
[grammar] ~208-~208: There might be a mistake here.
Context: ...-oriented): Problem-solving instructions - Explanation (understanding-oriented): ...
(QB_NEW_EN)
[grammar] ~209-~209: There might be a mistake here.
Context: ...: Clarification and discussion of topics - Reference (information-oriented): Tech...
(QB_NEW_EN)
[grammar] ~285-~285: There might be a mistake here.
Context: ...itional imperatives: "If you want X, do Y" - Address users directly: "Configure...", ...
(QB_NEW_EN)
[grammar] ~286-~286: There might be a mistake here.
Context: ..."Configure...", "Create...", "Deploy..." - Focus on practical tasks without digress...
(QB_NEW_EN)
[grammar] ~300-~300: There might be a mistake here.
Context: ...uced - Use domain-relevant language from user perspective (playbooks, branches, ...
(QB_NEW_EN)
[grammar] ~302-~302: There might be a mistake here.
Context: ...ing conventions - Validate accuracy against latest Infrahub version - **Follow mark...
(QB_NEW_EN)
[grammar] ~308-~308: There might be a mistake here.
Context: ...sync and sync examples where applicable using Tabs component - Provide realistic exam...
(QB_NEW_EN)
docs/docs/python-sdk/guides/client.mdx
[grammar] ~4-~4: There might be a mistake here.
Context: ...ient --- import Tabs from '@theme/Tabs'; import TabItem from '@theme/TabItem'; #...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...## Prerequisites - Python 3.9 or higher - Infrahub SDK installed (`pip install inf...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...rahub-sdkorpoetry add infrahub-sdk`) - Access to an Infrahub instance (local or...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...o an Infrahub instance (local or remote) - Valid credentials (API token or username...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...password. :::info Configuration Methods You can configure the client in two ways...
(QB_NEW_EN)
[grammar] ~143-~143: There might be a mistake here.
Context: ...ssword For interactive authentication, use username and password. The SDK automati...
(QB_NEW_EN)
[style] ~160-~160: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Item> We need to export the address, username and passwo...
(REP_NEED_TO_VB)
[style] ~190-~190: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Item> We need to export the address, username and passwo...
(REP_NEED_TO_VB)
[grammar] ~262-~262: There might be a mistake here.
Context: ...ello_world.py ``` :::danger File Naming Avoid naming your script test.py as th...
(QB_NEW_EN)
[grammar] ~263-~263: There might be a mistake here.
Context: ... conflict with Python's testing modules. ::: 2. Add the following code to `hello...
(QB_NEW_EN)
| # Run tests for specific modules | ||
| poetry run pytest tests/unit/test_client.py | ||
| poetry run pytest tests/unit/test_node.py | ||
|
|
||
| # Run with verbose output | ||
| poetry run pytest -v tests/unit/ | ||
|
|
||
| # Run with parallel execution | ||
| poetry run pytest -n 4 tests/unit/ | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix test paths to match repository layout; current commands will 404.
The repo organizes many unit tests under tests/unit/sdk/, and node tests referenced in this PR live in integration. Update the examples so they work out-of-the-box.
-# Run tests for specific modules
-poetry run pytest tests/unit/test_client.py
-poetry run pytest tests/unit/test_node.py
+# Run tests for specific modules
+# Unit
+poetry run pytest tests/unit/sdk/test_client.py
+# Integration
+poetry run pytest tests/integration/test_node.py📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Run tests for specific modules | |
| poetry run pytest tests/unit/test_client.py | |
| poetry run pytest tests/unit/test_node.py | |
| # Run with verbose output | |
| poetry run pytest -v tests/unit/ | |
| # Run with parallel execution | |
| poetry run pytest -n 4 tests/unit/ | |
| ``` | |
| # Run tests for specific modules | |
| # Unit | |
| poetry run pytest tests/unit/sdk/test_client.py | |
| # Integration | |
| poetry run pytest tests/integration/test_node.py | |
| # Run with verbose output | |
| poetry run pytest -v tests/unit/ | |
| # Run with parallel execution | |
| poetry run pytest -n 4 tests/unit/ |
🤖 Prompt for AI Agents
In CLAUDE.md around lines 35 to 44, the example pytest commands reference test
paths that no longer exist; update them to point to the current repository
layout: use tests/unit/sdk/test_client.py and tests/unit/sdk/test_node.py (or
the appropriate file names under tests/unit/sdk/) for unit tests, and use
tests/integration/ (or the specific integration test files, e.g.
tests/integration/test_node.py) for node/integration tests; also update the
directory-wide examples to poetry run pytest -v tests/unit/sdk/ and poetry run
pytest -n 4 tests/integration/ so the sample commands work out-of-the-box with
the repo structure.
| <Tabs groupId="env-config"> | ||
| <TabItem value="Code" default> | ||
|
|
||
| ```python | ||
| from infrahub_sdk import InfrahubClient, Config | ||
| config = Config(address="http://localhost:8000", api_token="token", default_branch="other-branch") | ||
| client_other_branch = InfrahubClient(config=config) | ||
|
|
||
| tag_other_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED") | ||
| tag_main_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main") | ||
| ``` | ||
|
|
||
| </TabItem> | ||
| <TabItem value="Environment"> | ||
|
|
||
| ```bash | ||
| export INFRAHUB_ADDRESS="http://localhost:8000" | ||
| export INFRAHUB_API_TOKEN="token" | ||
| export INFRAHUB_DEFAULT_BRANCH="other-branch" | ||
| ``` | ||
|
|
||
| ```python | ||
| from infrahub_sdk import InfrahubClient | ||
| client_other_branch = InfrahubClient() | ||
|
|
||
| tag_other_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED") | ||
| tag_main_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main") | ||
| ``` | ||
|
|
||
| </TabItem> | ||
| </Tabs> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add Sync examples and wrap async awaits to make the snippet runnable
Step 3 only shows async usage and uses bare await at top level, which is not directly runnable. Per the docs guidelines, include both async and sync examples via Tabs. Also, wrap async calls in a small coroutine (or show a context manager) to avoid confusing copy/paste.
Apply this diff to restructure Step 3:
-<Tabs groupId="env-config">
-<TabItem value="Code" default>
-
-```python
-from infrahub_sdk import InfrahubClient, Config
-config = Config(address="http://localhost:8000", api_token="token", default_branch="other-branch")
-client_other_branch = InfrahubClient(config=config)
-
-tag_other_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED")
-tag_main_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main")
-```
-
-</TabItem>
-<TabItem value="Environment">
+<Tabs groupId="async-sync">
+ <TabItem value="Async" default>
+ <Tabs groupId="env-config">
+ <TabItem value="Code" default>
+
+```python
+import asyncio
+from infrahub_sdk import InfrahubClient, Config
+
+async def main():
+ config = Config(address="http://localhost:8000", api_token="token", default_branch="other-branch")
+ async with InfrahubClient(config=config) as client_other_branch:
+ tag_other_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED")
+ tag_main_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main")
+ print(tag_other_branch, tag_main_branch)
+
+asyncio.run(main())
+```
+ </TabItem>
+ <TabItem value="Environment">
+
+```bash
+export INFRAHUB_ADDRESS="http://localhost:8000"
+export INFRAHUB_API_TOKEN="token"
+export INFRAHUB_DEFAULT_BRANCH="other-branch"
+```
+
+```python
+import asyncio
+from infrahub_sdk import InfrahubClient
+
+async def main():
+ async with InfrahubClient() as client_other_branch:
+ tag_other_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED")
+ tag_main_branch = await client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main")
+ print(tag_other_branch, tag_main_branch)
+
+asyncio.run(main())
+```
+ </TabItem>
+ </Tabs>
+ </TabItem>
+ <TabItem value="Sync">
+ <Tabs groupId="env-config">
+ <TabItem value="Code" default>
+
+```python
+from infrahub_sdk import InfrahubClientSync, Config
+config = Config(address="http://localhost:8000", api_token="token", default_branch="other-branch")
+client_other_branch = InfrahubClientSync(config=config)
+
+tag_other_branch = client_other_branch.get(kind="BuiltinTag", name__value="RED")
+tag_main_branch = client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main")
+print(tag_other_branch, tag_main_branch)
+```
+ </TabItem>
+ <TabItem value="Environment">
+
+```bash
+export INFRAHUB_ADDRESS="http://localhost:8000"
+export INFRAHUB_API_TOKEN="token"
+export INFRAHUB_DEFAULT_BRANCH="other-branch"
+```
+
+```python
+from infrahub_sdk import InfrahubClientSync
+client_other_branch = InfrahubClientSync()
+
+tag_other_branch = client_other_branch.get(kind="BuiltinTag", name__value="RED")
+tag_main_branch = client_other_branch.get(kind="BuiltinTag", name__value="RED", branch="main")
+print(tag_other_branch, tag_main_branch)
+```
+ </TabItem>
+ </Tabs>
+ </TabItem>
+</Tabs>🤖 Prompt for AI Agents
In docs/docs/python-sdk/guides/client.mdx around lines 216 to 246, the current
snippet uses bare await at top level and only shows async usage; replace it with
a Tabs block that provides both Async and Sync examples: wrap async calls in an
async def main() and call asyncio.run(main()), use an async context manager
(async with InfrahubClient(...)) for the async examples, and keep the env-config
Tab content inside each Async/Sync group; add equivalent synchronous examples
using InfrahubClientSync (no awaits) and ensure each code Tab prints or
otherwise uses the returned values so the snippets are runnable and
copy/paste-ready.
| 3. Export the necessary environment variables: | ||
|
|
||
| ```bash | ||
| export INFRAHUB_API_TOKEN="06438eb2-8019-4776-878c-0941b1f1d1ec" # This is the default token | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid claiming or publishing a “default token”; use a placeholder and show expected output as an example
Hardcoding a token and stating it’s a default may be misleading. Prefer a placeholder and present the output as an example rather than an exact value.
-```bash
-export INFRAHUB_API_TOKEN="06438eb2-8019-4776-878c-0941b1f1d1ec" # This is the default token
-```
+```bash
+# Replace with your API token
+export INFRAHUB_API_TOKEN="<your-api-token>"
+```
@@
-:::success
-`Successfully connected to Infrahub! Found 1 account(s)`
-:::
+:::success
+Example:
+`Successfully connected to Infrahub! Found 1 account(s)`
+:::Also applies to: 328-333
🤖 Prompt for AI Agents
In docs/docs/python-sdk/guides/client.mdx around lines 316-321 (and similarly
328-333), remove the hardcoded "default" API token and replace it with a
placeholder, and change the success block to indicate the output is an example;
specifically, update the export command to use a placeholder like
<your-api-token> and add a preceding comment line saying "Replace with your API
token", and modify the success note to prefix with "Example:" so the shown
output is clearly illustrative rather than an actual result.
| def test_not_password_authentication() -> None: | ||
| config = Config() | ||
| assert not config.password_authentication | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Mirror check for the negative case
Same note as above; ensure the attribute is boolean.
🏁 Script executed:
#!/bin/bash
# Same verification as above; included here for convenience.
rg -n -C2 -P '@property\s*\n\s*def\s+password_authentication|def\s+password_authentication\s*\(self\)' infrahub_sdk/config.pyLength of output: 245
Refine the negative‐case assertion to enforce a strict boolean check
Please update the test in tests/unit/sdk/test_config.py to both verify the value is False and that it’s a bool. For example, replace or augment:
# Line 27
assert not config.password_authenticationwith one of the following:
• Enforce identity (value and type):
assert config.password_authentication is False• Or split into two explicit checks:
assert isinstance(config.password_authentication, bool)
assert not config.password_authenticationThis change will ensure the property returns a strict boolean rather than just a falsy value.
🤖 Prompt for AI Agents
In tests/unit/sdk/test_config.py around lines 26 to 29, the test currently uses
a falsy check (assert not config.password_authentication); update it to enforce
a strict boolean False by replacing the assertion with either an identity check
(assert config.password_authentication is False) or two assertions (assert
isinstance(config.password_authentication, bool) followed by assert not
config.password_authentication) so the test ensures the property is actually a
bool and specifically False.
Replaces #502 with resolved merge conflicts.
Summary by CodeRabbit