Skip to content

Fix invalid-argument-type violations#841

Merged
ogenstad merged 1 commit intoinfrahub-developfrom
pog-timestamp-argument
Feb 19, 2026
Merged

Fix invalid-argument-type violations#841
ogenstad merged 1 commit intoinfrahub-developfrom
pog-timestamp-argument

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Feb 19, 2026

Why

Remove typing exceptions and fix violations

What changed

Cleanup of argument types in timestamp module.

Summary by CodeRabbit

  • Bug Fixes

    • Improved timestamp parsing consistency.
  • Chores

    • Standardized code quality checks across the codebase.

@cloudflare-workers-and-pages
Copy link

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: b417aaf
Status: ✅  Deploy successful!
Preview URL: https://64a5f016.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-timestamp-argument.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@                Coverage Diff                @@
##           infrahub-develop     #841   +/-   ##
=================================================
  Coverage             80.59%   80.59%           
=================================================
  Files                   118      118           
  Lines                 10235    10235           
  Branches               1538     1538           
=================================================
  Hits                   8249     8249           
  Misses                 1456     1456           
  Partials                530      530           
Flag Coverage Δ
integration-tests 40.40% <0.00%> (+<0.01%) ⬆️
python-3.10 51.81% <100.00%> (ø)
python-3.11 51.81% <100.00%> (ø)
python-3.12 51.81% <100.00%> (+0.01%) ⬆️
python-3.13 51.79% <100.00%> (ø)
python-3.14 53.48% <100.00%> (-0.02%) ⬇️
python-filler-3.12 23.97% <0.00%> (ø)

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

Files with missing lines Coverage Δ
infrahub_sdk/timestamp.py 82.88% <100.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 February 19, 2026 12:34
@ogenstad ogenstad requested a review from a team as a code owner February 19, 2026 12:34
@coderabbitai
Copy link

coderabbitai bot commented Feb 19, 2026

Walkthrough

This pull request modifies the infrahub_sdk/timestamp.py file and its configuration. In the _parse_string function, the code now passes explicit keyword arguments (seconds, minutes, hours) to the subtract method, each with a default value of 0.0 if not provided, replacing the previous dynamic keyword expansion approach. Additionally, a type-check override for this file in pyproject.toml has been removed, including its associated invalid-argument-type ignore rule, subjecting the file to default type-checking rules.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing invalid-argument-type violations in the codebase.
Description check ✅ Passed The description covers the 'Why' and 'What changed' sections but is minimal and lacks detail on testing, impact, and other recommended sections.

✏️ 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
infrahub_sdk/timestamp.py (1)

16-19: Nit: Typo in SubstractParams class name.

Substract should be Subtract. While pre-existing and not introduced by this PR, since you're already touching the file it may be worth correcting it now.

✏️ Proposed rename
-class SubstractParams(TypedDict):
+class SubtractParams(TypedDict):
     seconds: NotRequired[float]
     minutes: NotRequired[float]
     hours: NotRequired[float]

And update the usage at line 83:

-    params: SubstractParams = {}
+    params: SubtractParams = {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infrahub_sdk/timestamp.py` around lines 16 - 19, Rename the misspelled
TypedDict class SubstractParams to SubtractParams and update all references to
it (e.g., type annotations or usages where SubstractParams is referenced such as
the usage near the timestamp handling code) to use SubtractParams; ensure
imports, type hints, and any stringified type names are updated consistently and
run type checks to catch remaining references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@infrahub_sdk/timestamp.py`:
- Around line 16-19: Rename the misspelled TypedDict class SubstractParams to
SubtractParams and update all references to it (e.g., type annotations or usages
where SubstractParams is referenced such as the usage near the timestamp
handling code) to use SubtractParams; ensure imports, type hints, and any
stringified type names are updated consistently and run type checks to catch
remaining references.

@ogenstad ogenstad merged commit a48fa78 into infrahub-develop Feb 19, 2026
21 checks passed
@ogenstad ogenstad deleted the pog-timestamp-argument branch February 19, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments