Fix return annotations and changes for early returns #644
Conversation
Walkthroughpyproject.toml removed a per-file ignore for ANN201, causing the linter to enforce return type annotations. The changes add explicit return type hints and necessary typing imports (e.g., Generator, AsyncGenerator, Callable, Any) across multiple test fixtures and test functions in the tests/ tree. No runtime logic, control flow, or external behavior was modified; edits are limited to type annotations and minor formatting. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (12)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code graph analysis (2)tests/integration/test_infrahub_client.py (2)
tests/fixtures/integration/test_infrahubctl/tags_transform/tags_transform.py (1)
⏰ 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). (6)
🔇 Additional comments (14)
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: |
0aa08c2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://48627493.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-return-annotations-to-in.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #644 +/- ##
====================================================
+ Coverage 75.56% 75.60% +0.04%
====================================================
Files 113 113
Lines 9514 9522 +8
Branches 1448 1452 +4
====================================================
+ Hits 7189 7199 +10
+ Misses 1838 1832 -6
- Partials 487 491 +4
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
🧹 Nitpick comments (5)
tests/integration/test_infrahub_client.py (1)
37-37: Consider being more specific with the AsyncGenerator type.The fixture yields without a value (line 40), so
AsyncGenerator[None, None]would be more precise than bareAsyncGenerator.Apply this diff for more precise typing:
- async def set_pagination_size3(self, client: InfrahubClient) -> AsyncGenerator: + async def set_pagination_size3(self, client: InfrahubClient) -> AsyncGenerator[None, None]:tests/conftest.py (1)
3-3: Generator-based fixture annotations look good; optional tightening of typesUsing
collections.abc.Generatorforevent_loopandclean_env_varsmatches the recommended generator-style override pattern forevent_loopand should not change behavior.(newreleases.io)If you want stricter typing, you could parameterize the generators explicitly, e.g.:
def event_loop() -> Generator[asyncio.AbstractEventLoop, None, None]: ... def clean_env_vars() -> Generator[None, None, None]: ...to avoid implicit
Anyfor the send/return types.(docs.python.org)Also applies to: 15-15, 30-30
tests/unit/sdk/graphql/conftest.py (1)
2-2: GraphQL query/input fixtures: annotations and inlined returns are consistentImporting
Anyand annotating these fixtures asdict[str, Any]is consistent with their heterogeneous JSON-like payloads, and returning the dict literals directly removes unnecessary local variables without changing behavior. If you find yourself repeating this pattern elsewhere, you could optionally introduce a small alias likeJSONDict = dict[str, Any]for reuse, but what you have here is already clear.(typing.python.org)Also applies to: 18-26, 29-37, 40-49, 53-61, 65-76, 80-90, 94-103
tests/unit/sdk/conftest.py (2)
96-102: Callable-based helper fixtures are correctly typed; consider tiny optimizationThe new return types
replace_async_return_annotation(...) -> Callable[[str], str]replace_async_parameter_annotations(...) -> Callable[[Mapping[str, Parameter]], list[tuple[str, str]]]replace_sync_return_annotation(...) -> Callable[[str], str]replace_sync_parameter_annotations(...) -> Callable[[Mapping[str, Parameter]], list[tuple[str, str]]]all match the behavior of the inner functions and make the intent of these fixtures clear.(typing.python.org)
If you ever care about micro-optimizations, you could precompute the reverse
replacementsdict forreplace_sync_return_annotationonce in the outer fixture instead of rebuilding it on every call; functionally it’s fine as-is and probably not performance‑critical in tests.Also applies to: 106-118, 122-129, 133-145
305-353: Async data fixtures now explicitly returndict[str, Any]; behavior unchangedAcross the various
location_*_data*,rfile_userdata*,tag_*_data*,ipam_ipprefix_data,address_data,device_data, andartifact_definition_datafixtures, the changes are limited to:
- adding
-> dict[str, Any]return annotations, and- returning the dict literals directly instead of via an intermediate variable.
Given these are JSON/GraphQL-style payloads with mixed value types,
dict[str, Any]is a reasonable and consistent choice here, and the refactor should be behavior‑preserving.(typing.python.org)If you want to DRY things up later, a shared alias (e.g.
JSONDict = dict[str, Any]) or a small helper factory could reduce repetition, but that’s purely optional.Also applies to: 355-419, 422-457, 461-520, 524-559, 563-638, 642-661, 678-701, 705-728, 732-755, 759-784, 788-813, 817-842, 979-1043, 1175-1210, 1264-1416, 1437-1467
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
pyproject.toml(0 hunks)tests/conftest.py(3 hunks)tests/fixtures/integration/test_infrahubctl/tags_transform/tags_transform.py(1 hunks)tests/integration/test_infrahub_client.py(2 hunks)tests/unit/ctl/conftest.py(2 hunks)tests/unit/ctl/test_render_app.py(1 hunks)tests/unit/ctl/test_transform_app.py(2 hunks)tests/unit/sdk/conftest.py(23 hunks)tests/unit/sdk/graphql/conftest.py(6 hunks)tests/unit/sdk/graphql/test_plugin.py(2 hunks)tests/unit/sdk/test_repository.py(2 hunks)tests/unit/sdk/test_schema.py(1 hunks)
💤 Files with no reviewable changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (2)
tests/integration/test_infrahub_client.py (2)
tests/unit/sdk/conftest.py (1)
client(32-33)infrahub_sdk/client.py (1)
InfrahubClient(318-1688)
tests/fixtures/integration/test_infrahubctl/tags_transform/tags_transform.py (1)
infrahub_sdk/ctl/cli_commands.py (1)
transform(302-360)
⏰ 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 (13)
tests/unit/sdk/test_repository.py (2)
2-2: LGTM!The import addition is correct and necessary for the return type annotation on line 14.
14-14: LGTM!The return type annotation
Generator[str]correctly describes this fixture, which yields a string path to a temporary directory.tests/unit/sdk/test_schema.py (1)
458-458: LGTM!The return type annotation
-> Noneis correct for this test function.tests/unit/ctl/test_transform_app.py (2)
8-8: LGTM!The import addition is correct and necessary for the return type annotation on line 31.
31-31: LGTM!The return type annotation
Generator[str]correctly describes this fixture, which yields a string path to a temporary directory.tests/fixtures/integration/test_infrahubctl/tags_transform/tags_transform.py (1)
8-8: LGTM!The return type annotation
-> dict[str, str]accurately describes the return value, which is a dictionary with string keys and string values.tests/integration/test_infrahub_client.py (1)
3-3: LGTM!The import addition is correct and necessary for the return type annotation on line 37.
tests/unit/ctl/test_render_app.py (1)
87-89: LGTM!The multi-line formatting improves readability, and the return type annotation
-> Noneis correct for this test function.tests/unit/ctl/conftest.py (2)
1-2: LGTM!The import addition is correct and necessary for the return type annotation on line 68.
68-77: LGTM!The return type annotation
-> dict[str, Any]accurately describes the fixture's return value, and the direct return is cleaner than using an intermediate variable.tests/unit/sdk/graphql/test_plugin.py (2)
38-38: LGTM!The return type annotation
-> Noneis correct for this test function.
51-53: LGTM!The multi-line formatting improves readability, and the return type annotation
-> Noneis correct for this test function.tests/unit/sdk/conftest.py (1)
5-5: Updated type imports align with usage and modern typing stylePulling
AsyncGenerator,Callable, andMappingfromcollections.abcandAnyfromtypingmatches current recommendations (PEP 585 and related typing docs) and fits how they’re used throughout this module.(peps.python.org)Also applies to: 9-9
ogenstad
force-pushed
the
pog-return-annotations-to-infrahub-develop-20251118
branch
from
22a9811 to
0aa08c2
Compare
ogenstad
deleted the
pog-return-annotations-to-infrahub-develop-20251118
branch
Summary by CodeRabbit
Tests
Chores