Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Nov 18, 2025

Summary by CodeRabbit

  • Refactor
    • Optimized internal code patterns for consistency and performance by standardizing membership checks across the codebase.

@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

Walkthrough

This pull request standardizes membership checking throughout the SDK codebase by systematically replacing list and tuple literals with set literals in in and not in containment checks across multiple modules. Changes span HTTP status code validation, YAML file extension filtering, enum membership tests, and schema validation logic. Additionally, the pylint rule PLR6201 ("use a set literal when testing for membership") is enabled in pyproject.toml, enforcing this pattern going forward. The functional behavior of all checks remains equivalent; only the container type for membership operations is modified.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.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 directly and accurately reflects the primary change: replacing list/tuple literals with set literals for membership testing throughout 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-PLR6201-infrahub-develop-20251118

📜 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 8d6a51c.

📒 Files selected for processing (12)
  • infrahub_sdk/client.py (7 hunks)
  • infrahub_sdk/node/node.py (2 hunks)
  • infrahub_sdk/object_store.py (4 hunks)
  • infrahub_sdk/protocols_generator/generator.py (1 hunks)
  • infrahub_sdk/pytest_plugin/models.py (1 hunks)
  • infrahub_sdk/pytest_plugin/plugin.py (1 hunks)
  • infrahub_sdk/schema/__init__.py (1 hunks)
  • infrahub_sdk/spec/object.py (1 hunks)
  • infrahub_sdk/utils.py (1 hunks)
  • pyproject.toml (0 hunks)
  • tests/integration/test_infrahub_client.py (1 hunks)
  • tests/unit/sdk/test_node.py (1 hunks)
💤 Files with no reviewable changes (1)
  • pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
infrahub_sdk/node/node.py (1)
infrahub_sdk/schema/main.py (1)
  • RelationshipKind (28-36)
⏰ 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)
infrahub_sdk/utils.py (1)

171-171: LGTM! Clean refactor to set literal for membership check.

The change from list to set literal is functionally equivalent and follows Python best practices for membership testing.

infrahub_sdk/spec/object.py (1)

77-77: LGTM! Set literal for enum membership check.

The refactor is clean and maintains the same logic while following Python best practices.

infrahub_sdk/protocols_generator/generator.py (1)

53-60: LGTM! Set literal for blacklist membership check.

Using a set literal for the exclusion list is idiomatic and maintains equivalent functionality.

infrahub_sdk/pytest_plugin/models.py (1)

58-58: LGTM! Set literal for file extension check.

The change from tuple to set literal is appropriate and maintains the same behavior.

infrahub_sdk/pytest_plugin/plugin.py (1)

93-93: LGTM! Set literal for YAML file extension filtering.

Consistent with the change in models.py and follows the same pattern for file extension checks.

tests/integration/test_infrahub_client.py (1)

108-108: LGTM! Test assertion updated to use set literal.

The change maintains test correctness while aligning with the codebase-wide refactor to set-based membership checks.

infrahub_sdk/schema/__init__.py (1)

196-201: LGTM! Set literal for HTTP status code validation.

The multi-line set literal improves readability while maintaining the same validation logic. Good refactor.

infrahub_sdk/node/node.py (1)

744-744: LGTM! Set literals for relationship kind filtering.

Both the async (Line 744) and sync (Line 1367) implementations have been consistently refactored to use set literals. The changes maintain equivalent logic while following Python best practices.

Also applies to: 1367-1367

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

26-31: LGTM! Consistent use of set literals for membership testing.

The migration from tuple to set literals in these membership tests is functionally equivalent and aligns with best practices. Using sets for membership checks is semantically clearer since these represent collections where order doesn't matter.

infrahub_sdk/client.py (3)

302-302: LGTM! Improved membership test.

The change from tuple to set literal for the membership check is semantically clearer and aligns with the pylint rule PLR6201 being enabled in this PR.


959-959: LGTM! Consistent use of set literals for parameter validation.

The migration from tuple to set literals for checking the raise_for_error parameter values maintains identical functionality while improving consistency. This pattern is correctly applied across both async and sync implementations.

Also applies to: 1211-1211, 1820-1820, 2449-2449


973-973: LGTM! HTTP status code checks now use sets.

The change from list to set literal for HTTP status code membership checks is a good practice. While the performance difference is negligible for just two values, sets are semantically more appropriate for membership testing.

Also applies to: 1834-1834

infrahub_sdk/object_store.py (1)

36-36: LGTM! Consistent HTTP status checks across async and sync implementations.

The migration from list to set literals for HTTP status code membership checks is applied consistently across both the async (ObjectStore) and sync (ObjectStoreSync) classes. This aligns well with the same pattern used in infrahub_sdk/client.py.

Also applies to: 57-57, 84-84, 105-105


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

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8d6a51c
Status: ✅  Deploy successful!
Preview URL: https://39d8e683.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-plr6201-infrahub-develop.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

❌ Patch coverage is 31.25000% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/client.py 28.57% 3 Missing and 2 partials ⚠️
infrahub_sdk/object_store.py 0.00% 4 Missing ⚠️
infrahub_sdk/pytest_plugin/models.py 0.00% 0 Missing and 1 partial ⚠️
infrahub_sdk/schema/__init__.py 0.00% 0 Missing and 1 partial ⚠️
@@                Coverage Diff                @@
##           infrahub-develop     #642   +/-   ##
=================================================
  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% <12.50%> (ø)
python-3.10 48.88% <18.75%> (+0.02%) ⬆️
python-3.11 48.88% <18.75%> (+0.02%) ⬆️
python-3.12 48.84% <18.75%> (-0.03%) ⬇️
python-3.13 48.84% <18.75%> (ø)
python-3.9 48.84% <18.75%> (ø)
python-filler-3.12 24.29% <6.25%> (ø)

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

Files with missing lines Coverage Δ
infrahub_sdk/node/node.py 76.74% <ø> (ø)
infrahub_sdk/protocols_generator/generator.py 95.00% <ø> (ø)
infrahub_sdk/pytest_plugin/plugin.py 85.10% <100.00%> (ø)
infrahub_sdk/spec/object.py 84.79% <100.00%> (ø)
infrahub_sdk/utils.py 86.30% <100.00%> (ø)
infrahub_sdk/pytest_plugin/models.py 78.89% <0.00%> (ø)
infrahub_sdk/schema/__init__.py 67.52% <0.00%> (ø)
infrahub_sdk/object_store.py 55.29% <0.00%> (ø)
infrahub_sdk/client.py 68.22% <28.57%> (ø)
🚀 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 06:58
@ogenstad ogenstad requested a review from a team November 19, 2025 07:42
@ogenstad ogenstad merged commit 1dcbcec into infrahub-develop Nov 19, 2025
20 checks passed
@ogenstad ogenstad deleted the pog-PLR6201-infrahub-develop-20251118 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