Fix typing for substract params #540
Conversation
Walkthrough
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
Deploying infrahub-sdk-python with
|
| Latest commit: |
751b7da
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dcd02a58.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-substract-param-typing.infrahub-sdk-python.pages.dev |
|
Old Python :( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/timestamp.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
infrahub_sdk/timestamp.py
⏰ 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: integration-tests-latest-infrahub
🔇 Additional comments (1)
infrahub_sdk/timestamp.py (1)
89-89: Verify no remaining type errors
mypyisn’t available in this environment—please runmypy . --config-file pyproject.tomllocally to confirm there are no type errors.
infrahub_sdk/timestamp.py
Outdated
| import warnings | ||
| from datetime import datetime, timezone | ||
| from typing import Literal | ||
| from typing import Literal, NotRequired, TypedDict |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard NotRequired import for Python < 3.11
If the SDK supports 3.10 or earlier, this import will fail. Add a typing_extensions fallback.
-from typing import Literal, NotRequired, TypedDict
+from typing import Literal, TypedDict
+try: # Python 3.11+
+ from typing import NotRequired # type: ignore[attr-defined]
+except Exception: # Python <= 3.10
+ from typing_extensions import NotRequiredVerify supported versions:
🏁 Script executed:
#!/bin/bash
# Discover declared Python support
rg -nP 'python_requires\s*=' setup.cfg pyproject.toml setup.py 2>/dev/null || true
rg -nP 'Programming Language :: Python :: (3\.\d+)' setup.cfg pyproject.toml setup.py 2>/dev/null || trueLength of output: 318
🏁 Script executed:
#!/bin/bash
rg -nP 'requires-python' -C2 -n pyproject.toml || trueLength of output: 0
Guard NotRequired import for Python < 3.11
The SDK’s classifiers include Python 3.9 and 3.10 support, but NotRequired was only added in 3.11—this import will fail under 3.10. Add a typing_extensions fallback in infrahub_sdk/timestamp.py:
-from typing import Literal, NotRequired, TypedDict
+from typing import Literal, TypedDict
+try: # Python 3.11+
+ from typing import NotRequired # type: ignore[attr-defined]
+except ImportError: # Python ≤ 3.10
+ from typing_extensions import NotRequired📝 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.
| from typing import Literal, NotRequired, TypedDict | |
| from typing import Literal, TypedDict | |
| try: # Python 3.11+ | |
| from typing import NotRequired # type: ignore[attr-defined] | |
| except ImportError: # Python ≤ 3.10 | |
| from typing_extensions import NotRequired |
🤖 Prompt for AI Agents
In infrahub_sdk/timestamp.py around line 6, the direct import of NotRequired
from typing will break on Python <3.11; change the imports to try to import
NotRequired from typing and fall back to typing_extensions when it’s unavailable
(use a try/except ImportError or AttributeError to import NotRequired from
typing_extensions), keeping other typing imports intact and preserving
type-checker compatibility.
infrahub_sdk/timestamp.py
Outdated
| class SubstractParams(TypedDict): | ||
| seconds: NotRequired[int] | ||
| minutes: NotRequired[int] | ||
| hours: NotRequired[int] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Use float types and fix public name (SubtractParams); keep alias for backward compatibility
- Values assigned at Line 86 are floats.
- Library subtract(...) accepts floats for hours/minutes/seconds.
- Public type name is misspelled.
Apply:
-class SubstractParams(TypedDict):
- seconds: NotRequired[int]
- minutes: NotRequired[int]
- hours: NotRequired[int]
+class SubtractParams(TypedDict):
+ seconds: NotRequired[float]
+ minutes: NotRequired[float]
+ hours: NotRequired[float]
+
+# Backward-compat alias (to avoid breaking imports)
+SubstractParams = SubtractParamsTo find external uses of the misspelled name before/after the change:
🏁 Script executed:
#!/bin/bash
rg -n '\bSubstractParams\b' || true
rg -n '\bSubtractParams\b' || trueLength of output: 128
Rename SubstractParams to SubtractParams, switch to float, and add backward-compat alias
Apply:
-class SubstractParams(TypedDict):
- seconds: NotRequired[int]
- minutes: NotRequired[int]
- hours: NotRequired[int]
+class SubtractParams(TypedDict):
+ seconds: NotRequired[float]
+ minutes: NotRequired[float]
+ hours: NotRequired[float]
+
+# Backward-compat alias
+SubstractParams = SubtractParams🤖 Prompt for AI Agents
In infrahub_sdk/timestamp.py around lines 21 to 24, rename the TypedDict class
SubstractParams to the correctly spelled SubtractParams, change the types of
seconds, minutes, and hours from int to float (NotRequired[float]) so callers
can pass fractional values, and add a backward-compatible alias SubstractParams
= SubtractParams (preserve existing import behavior) so old code continues to
work while new code uses the corrected name and float types.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## develop #540 +/- ##
===========================================
- Coverage 75.94% 75.86% -0.09%
===========================================
Files 100 100
Lines 9329 8878 -451
Branches 1832 1740 -92
===========================================
- Hits 7085 6735 -350
+ Misses 1754 1668 -86
+ Partials 490 475 -15
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
ogenstad
force-pushed
the
pog-substract-param-typing
branch
from
c2737c7 to
751b7da
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrahub_sdk/timestamp.py (1)
22-26: Harden unit regex: prevent “ms”→minutes misparse; accept decimals/whitespace; case-insensitive.
Current patterns can read “5ms” as 5 minutes and reject decimals like “1.5h”.-REGEX_MAPPING = { - "seconds": r"(\d+)(s|sec|second|seconds)", - "minutes": r"(\d+)(m|min|minute|minutes)", - "hours": r"(\d+)(h|hour|hours)", -} +REGEX_MAPPING = { + "seconds": r"(\d+(?:\.\d+)?)\s*(?:s|sec|second|seconds)\b", + "minutes": r"(\d+(?:\.\d+)?)\s*(?:min|mins|minute|minutes|m)\b", + "hours": r"(\d+(?:\.\d+)?)\s*(?:h|hr|hour|hours)\b", +}And make searches case-insensitive (see Line 86):
- match = re.search(regex, value) + match = re.search(regex, value, re.IGNORECASE)
🧹 Nitpick comments (1)
infrahub_sdk/timestamp.py (1)
84-84: Update local annotation to the corrected type name.- params: SubstractParams = {} + params: SubtractParams = {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/timestamp.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
infrahub_sdk/timestamp.py
🧬 Code graph analysis (1)
infrahub_sdk/timestamp.py (1)
infrahub_sdk/exceptions.py (1)
TimestampFormatError(170-173)
⏰ 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). (7)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.9)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.12)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
infrahub_sdk/timestamp.py (4)
6-9: LGTM: typing_extensions.NotRequired import fixes Py≤3.10.
Good compatibility tweak; no runtime impact.
88-93: TypedDict population is fine and type-checker-friendly.
Explicit keys avoid dynamic-key TypedDict issues.
96-96: **LGTM: remove type ignore; call subtract(params) directly.
Cleaner and type-safe with the new TypedDict.
16-21: Rename misspelled public type to SubtractParams and add backward-compatible aliasrg found only local occurrences in infrahub_sdk/timestamp.py (class at L16, usage at L84). Rename the class, add an alias to preserve external imports, and update local references to the correct name.
-class SubstractParams(TypedDict): +class SubtractParams(TypedDict): seconds: NotRequired[float] minutes: NotRequired[float] hours: NotRequired[float] +# Backward-compat alias (preserve imports) +SubstractParams = SubtractParams
2 participants
Summary by CodeRabbit
New Features
New Features
Refactor
Documentation