Skip to content

--throttle should take a float, not an int#1736

Merged
rolandwalker merged 1 commit intomainfrom
RW/fix-float-throttle-option
Mar 24, 2026
Merged

--throttle should take a float, not an int#1736
rolandwalker merged 1 commit intomainfrom
RW/fix-float-throttle-option

Conversation

@rolandwalker
Copy link
Contributor

Description

Introduced in #1732.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

@rolandwalker rolandwalker self-assigned this Mar 23, 2026
@github-actions
Copy link

  1. Medium: --throttle now accepts non-finite values (inf, -inf, nan) and passes them to sleep()
    At mycli/main.py:2161, switching to type=float allows click to parse inf/-inf/nan. Those values flow to mycli/main.py:2706:

    • nan / -inf raise at runtime (after entering batch execution)
    • inf can block indefinitely
      Action: validate at parse time (math.isfinite(value) and value >= 0) via a callback or constrained type.
  2. Low: No regression tests for the behavior this PR changes
    I don’t see tests covering --throttle parsing/validation in test/test_main.py.
    Action: add CLI tests for:

    • --throttle 0.5 accepted and used
    • invalid values (nan, inf, negative) rejected with a clear CLI error before query execution.

No additional security issues were found beyond the non-finite input handling above.

@rolandwalker rolandwalker force-pushed the RW/fix-float-throttle-option branch from 5d01a98 to 51d72b7 Compare March 23, 2026 19:12
@rolandwalker rolandwalker merged commit 2775a15 into main Mar 24, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/fix-float-throttle-option branch March 24, 2026 09:25
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.

2 participants