Skip to content

IHS-183: Fix typing errors for protocols#763

Merged
ogenstad merged 10 commits into
developfrom
re-sb-20260119-fix-typing-for-protocols-ihs-183
Jun 8, 2026
Merged

IHS-183: Fix typing errors for protocols#763
ogenstad merged 10 commits into
developfrom
re-sb-20260119-fix-typing-for-protocols-ihs-183

Conversation

@solababs

@solababs solababs commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #697

Replace protocols with concrete classes to address type-abstract mypy errors

Summary by CodeRabbit

  • Refactor

    • Enhanced flexibility for related node handling to support duck-typing patterns.
    • Refactored internal class hierarchy for improved type safety and explicit error handling.
    • Updated schema validation logic for more robust type checking.
  • Chores

    • Updated Ruff development tool to latest version.
    • Removed unnecessary type-checking suppressions throughout codebase.
  • Tests

    • Added test coverage for schema name resolution.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 22, 2026

Copy link
Copy Markdown

Walkthrough

This pull request converts Protocol-based type definitions to concrete base classes and updates related type handling throughout the SDK. Changes include removing @runtime_checkable decorators from CoreNodeBase, CoreNode, and CoreNodeSync in protocols_base.py, converting them to concrete classes with NotImplementedError implementations. Additional updates address protocol handling in schema validation, related node type checking, and schema name resolution logic. Dependencies on runtime protocol attributes were replaced with subclass checks and explicit type casting. Version updates and type-ignore directives were also removed from several files.

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: fixing typing errors related to protocols, which aligns with the primary objective of converting protocol types to concrete classes.
Linked Issues check ✅ Passed All changes directly address issue #697 by converting protocols to concrete classes (CoreNodeBase, CoreNode, CoreNodeSync) and updating type checks/imports, eliminating mypy 'type-abstract' errors when using generated kinds.
Out of Scope Changes check ✅ Passed All changes are directly related to converting protocols to concrete classes and removing type-ignore directives. The Ruff version update in .pre-commit-config.yaml is a minor tooling maintenance update tangential to the main fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

cloudflare-workers-and-pages Bot commented Jan 22, 2026

Copy link
Copy Markdown

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 810a5c4
Status: ✅  Deploy successful!
Preview URL: https://62bebc14.infrahub-sdk-python.pages.dev
Branch Preview URL: https://re-sb-20260119-fix-typing-fo.infrahub-sdk-python.pages.dev

View logs

@codecov

codecov Bot commented Jan 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.14286% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/store.py 70.00% 1 Missing and 2 partials ⚠️
infrahub_sdk/schema/__init__.py 33.33% 1 Missing and 1 partial ⚠️
@@             Coverage Diff             @@
##           develop     #763      +/-   ##
===========================================
- Coverage    82.02%   81.85%   -0.18%     
===========================================
  Files          135      135              
  Lines        11841    11665     -176     
  Branches      1762     1761       -1     
===========================================
- Hits          9713     9548     -165     
+ Misses        1576     1571       -5     
+ Partials       552      546       -6     
Flag Coverage Δ
integration-tests 41.66% <14.28%> (-0.97%) ⬇️
python-3.10 55.25% <35.71%> (-0.66%) ⬇️
python-3.11 55.26% <35.71%> (-0.64%) ⬇️
python-3.12 55.25% <35.71%> (-0.66%) ⬇️
python-3.13 55.26% <35.71%> (-0.64%) ⬇️
python-3.14 55.24% <35.71%> (-0.65%) ⬇️
python-filler-3.12 22.34% <46.42%> (+0.37%) ⬆️

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

Files with missing lines Coverage Δ
infrahub_sdk/ctl/branch.py 81.48% <ø> (ø)
infrahub_sdk/node/attribute.py 100.00% <100.00%> (ø)
infrahub_sdk/node/related_node.py 92.44% <100.00%> (ø)
infrahub_sdk/protocols_base.py 77.60% <100.00%> (+4.16%) ⬆️
infrahub_sdk/testing/repository.py 77.77% <ø> (ø)
infrahub_sdk/schema/__init__.py 73.49% <33.33%> (+0.06%) ⬆️
infrahub_sdk/store.py 78.88% <70.00%> (-0.19%) ⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ogenstad ogenstad force-pushed the re-sb-20260119-fix-typing-for-protocols-ihs-183 branch from 6c5bd98 to 99ce971 Compare June 8, 2026 07:00
@ogenstad ogenstad requested a review from a team as a code owner June 8, 2026 07:00

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="infrahub_sdk/node/related_node.py">

<violation number="1" location="infrahub_sdk/node/related_node.py:50">
P2: `hasattr(data, "_schema")` is an overly broad peer check and can misclassify non-node objects, causing runtime attribute errors.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

self._peer = data
# Check for InfrahubNodeBase instances using duck-typing (_schema attribute)
# to avoid circular imports, or CoreNodeBase instances
if isinstance(data, CoreNodeBase) or hasattr(data, "_schema"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: hasattr(data, "_schema") is an overly broad peer check and can misclassify non-node objects, causing runtime attribute errors.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/node/related_node.py, line 50:

<comment>`hasattr(data, "_schema")` is an overly broad peer check and can misclassify non-node objects, causing runtime attribute errors.</comment>

<file context>
@@ -45,8 +45,10 @@ def __init__(self, branch: str, schema: RelationshipSchemaAPI, data: Any | dict,
-            self._peer = data
+        # Check for InfrahubNodeBase instances using duck-typing (_schema attribute)
+        # to avoid circular imports, or CoreNodeBase instances
+        if isinstance(data, CoreNodeBase) or hasattr(data, "_schema"):
+            self._peer = cast("InfrahubNodeBase | CoreNodeBase", data)
             for prop in self._properties:
</file context>

@ogenstad ogenstad requested review from a team and removed request for a team June 8, 2026 17:27
@ogenstad

ogenstad commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Rebased and pushed some changes to account for a new issue with numberpools also ran a test pipeline for this against infrahub, but based on the infrahub-develop branch in opsmill/infrahub#9498.

@ogenstad ogenstad merged commit 94559a0 into develop Jun 8, 2026
21 checks passed
@ogenstad ogenstad deleted the re-sb-20260119-fix-typing-for-protocols-ihs-183 branch June 8, 2026 19:04
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