Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Nov 18, 2025

Use specific ignore types where applicable instead of ignoring everything.

Summary by CodeRabbit

  • Refactor

    • Improved type hint specificity and static type checking across the codebase.
  • Chores

    • Updated build configuration and removed outdated code quality rules.

@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

Walkthrough

This pull request refines type checking and linting configurations across the codebase. Changes include narrowing generic type ignore comments to specific ignore codes (such as return-value, union-attr, and attr-defined) in multiple files to improve static type analysis precision. In infrahub_sdk/query_groups.py, the unused_member_ids variable is converted from a set to a list structure while preserving semantic content. The Ruff configuration in pyproject.toml removes the "PGH" rule from the ignore list. A test expectation comment is updated for consistency in formatting style.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Use specific rule codes when using noqa' directly aligns with the primary objective of the PR, which is to use specific ignore types instead of generic ignores across the codebase.
✨ 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 pog-pygrep-hooks

📜 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 21d565e and 3cf549f.

📒 Files selected for processing (8)
  • infrahub_sdk/config.py (1 hunks)
  • infrahub_sdk/query_groups.py (2 hunks)
  • pyproject.toml (0 hunks)
  • tests/__init__.py (1 hunks)
  • tests/integration/conftest.py (1 hunks)
  • tests/unit/__init__.py (1 hunks)
  • tests/unit/sdk/conftest.py (13 hunks)
  • tests/unit/sdk/test_template.py (1 hunks)
💤 Files with no reviewable changes (1)
  • pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (2)
infrahub_sdk/query_groups.py (1)
infrahub_sdk/node/relationship.py (1)
  • peer_ids (44-45)
tests/unit/sdk/conftest.py (1)
infrahub_sdk/schema/main.py (3)
  • NodeSchema (307-309)
  • convert_api (284-285)
  • convert_api (308-309)
⏰ 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 (8)
tests/integration/conftest.py (1)

251-251: LGTM – cleanup of commented code.

This line is entirely commented out (inside a commented fixture), so removing the type ignore directive has no functional impact. The change aligns with the PR's goal of cleaning up type annotations.

tests/unit/sdk/conftest.py (1)

177-177: LGTM – improved type checking.

Removing the generic # type: ignore directives from these NodeSchema(**data).convert_api() calls indicates that the type checker now properly infers the return types. This tightens type checking without any functional changes.

Also applies to: 219-219, 284-284, 298-298, 682-682, 896-896, 920-920, 944-944, 957-957, 989-989, 1109-1109, 1168-1168, 1277-1277, 1451-1451

tests/__init__.py (1)

6-7: LGTM – more precise type ignores.

Narrowing the generic # type: ignore to specific # type: ignore[attr-defined] is best practice. It clearly documents that the ignore is needed because attributes are being dynamically added to the builtins module.

infrahub_sdk/config.py (1)

176-176: LGTM – narrowed type ignore.

Specifying # type: ignore[return-value] instead of a generic ignore makes the suppression more precise and maintainable. The comment above provides clear context for why the ignore is necessary.

tests/unit/sdk/test_template.py (1)

223-223: LGTM – standardized noqa format.

The change from # noqa E501 to # noqa: E501 adopts the standard format with a colon. The test expectation correctly reflects this formatting improvement.

tests/unit/__init__.py (1)

5-5: LGTM – specific type ignore.

Using # type: ignore[attr-defined] instead of a generic ignore clearly documents why the suppression is needed for this dynamic attribute assignment on the builtins module.

infrahub_sdk/query_groups.py (2)

171-171: LGTM – corrected type mismatch.

The change from set(...) - set(...) to list(set(...) - set(...)) correctly aligns with the type annotation unused_member_ids: list[str] | None at line 22. The narrowed # type: ignore[union-attr] is also more precise.

Note: Membership testing (member.id in self.unused_member_ids at line 111) becomes O(n) with a list vs O(1) with a set, but this is unlikely to be a concern given the expected small collection sizes.


265-265: LGTM – corrected type mismatch (sync version).

Same as the async version at line 171: correctly converts the set difference to a list to match the type annotation, and uses a more specific type ignore directive.


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

cloudflare-workers-and-pages bot commented Nov 18, 2025

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3cf549f
Status: ✅  Deploy successful!
Preview URL: https://1a7ec445.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-pygrep-hooks.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/query_groups.py 0.00% 2 Missing ⚠️
@@                Coverage Diff                @@
##           infrahub-develop     #641   +/-   ##
=================================================
  Coverage             75.56%   75.56%           
=================================================
  Files                   113      113           
  Lines                  9514     9514           
  Branches               1448     1448           
=================================================
  Hits                   7189     7189           
  Misses                 1838     1838           
  Partials                487      487           
Flag Coverage Δ
integration-tests 34.96% <33.33%> (ø)
python-3.10 48.88% <33.33%> (+0.02%) ⬆️
python-3.11 48.88% <33.33%> (+0.02%) ⬆️
python-3.12 48.86% <33.33%> (ø)
python-3.13 48.84% <33.33%> (ø)
python-3.9 48.84% <33.33%> (ø)
python-filler-3.12 24.29% <33.33%> (ø)

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

Files with missing lines Coverage Δ
infrahub_sdk/config.py 90.15% <100.00%> (ø)
infrahub_sdk/query_groups.py 60.60% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ogenstad ogenstad marked this pull request as ready for review November 19, 2025 07:30
@ogenstad ogenstad requested a review from a team November 19, 2025 07:42
@ogenstad ogenstad merged commit a2d9569 into infrahub-develop Nov 19, 2025
20 checks passed
@ogenstad ogenstad deleted the pog-pygrep-hooks branch November 19, 2025 09:16
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.

3 participants