Skip to content

Declare limits as a direct dependency - #2407

Merged
Flix6x merged 4 commits into
mainfrom
fix/declare-rate-limiting-deps
Aug 14, 2026
Merged

Declare limits as a direct dependency#2407
Flix6x merged 4 commits into
mainfrom
fix/declare-rate-limiting-deps

Conversation

@Flix6x

@Flix6x Flix6x commented Aug 7, 2026

Copy link
Copy Markdown
Member

What

flexmeasures/utils/validation_utils.py does import limits (to validate rate limits via limits.parse), but limits was not declared in pyproject.toml. This adds it, with the lock-file edge to match.

Investigation, and what the actual defect is

The report that prompted this was two developer venvs failing with ModuleNotFoundError: No module named 'limits' and then No module named 'flask_limiter' after merging main, with the suspicion that PR #2306 had left both packages undeclared.

That is not what happened, and it is worth writing down:

  • Flask-Limiter is declared. PR Rate limiting plans for the API, and for scheduling and forecasting triggers in particular #2306 added "Flask-Limiter[redis]>=4.0" to pyproject.toml itself (git log -S 'Flask-Limiter' -- pyproject.toml points straight at fecf13a96). So the second error was a stale-venv symptom, not a packaging gap — those venvs simply had not been re-synced after the merge.
  • limits is genuinely undeclared, but it still installs. Flask-Limiter 4.1.1 requires limits>=3.13, so a fresh install from the declared dependencies does pull limits in transitively. I verified this: a clean venv installed from pyproject.toml on unmodified main gets limits 5.8.0.

So this is not a user-visible breakage, and I don't want to claim it is. What it is is a latent packaging hazard: we import a module we do not declare, and our only claim on it is that a third party happens to depend on it. If Flask-Limiter ever drops, renames or makes optional its limits dependency, import flexmeasures breaks with no signal from our own metadata — and nothing in CI would catch it, since CI installs with uv sync --frozen, which resolves purely from uv.lock, where limits is pinned regardless of who asked for it. The declared-dependency path is never exercised.

What changed

  • pyproject.toml: added "limits>=3.13", next to Flask-Limiter. The floor is Flask-Limiter's own requirement — we only use limits.parse, which is long-standing, so there is no reason to demand more.
  • uv.lock: the new flexmeasures -> limits edge and its specifier. No resolved version changedlimits 5.8.0 was already locked as a transitive dependency, so the diff is two lines. (I hand-applied it rather than committing a full uv lock regeneration, which would have rewritten ~1500 lines of environment-marker serialization with zero version changes. uv lock --check passes on the result, as does uv sync --frozen.)
  • documentation/changelog.rst: this PR appended to the rate limiting entry of Rate limiting plans for the API, and for scheduling and forecasting triggers in particular #2306, which introduced the rate limiting this dependency serves — no entry of its own.

Only limits is added. Flask-Limiter needs no change, and adding it would be a no-op.

Verification

The meaningful test here is a lock-free install, since a passing uv sync proves nothing. In a fresh Python 3.12 venv, installed from the declared dependencies only (uv pip install ., no uv.lock):

limits 5.8.0
flask_limiter 4.1.1
import flexmeasures OK: 1.0.0.dev111+gf3319e0f6
flexmeasures declares limits: ['limits>=3.13']

validate_rate_limit("500 per minute") and importing flexmeasures.api.common.rate_limiting both succeed.

Unrelated snag worth flagging

While doing that clean-room install I hit a pre-existing resolution problem on main, unrelated to this PR: resolving from pyproject.toml without the lock backtracks numba all the way to 0.47.0 (via darts), which then tries to build llvmlite 0.36.0 and fails on Python 3.12. I worked around it locally with numba>=0.61 to complete the verification. Same behaviour with and without this PR's change, so it is not a regression here, but it does mean the declared dependency set is not currently installable unaided — which is exactly the sort of thing a uv sync --frozen CI never sees. Probably worth a separate issue and a numba floor.

Flix6x and others added 2 commits August 7, 2026 12:04
flexmeasures/utils/validation_utils.py imports `limits` directly (to validate
rate limits with limits.parse), but PR #2306 only declared Flask-Limiter.
`limits` therefore reached us as a transitive dependency of Flask-Limiter,
which works today but is not something we should rely on: nothing stops
Flask-Limiter from dropping or renaming that dependency, and it makes
`uv sync --no-deps`-style or vendored installs fragile.

So declare it explicitly, with Flask-Limiter's own floor (>=3.13), since we
only use the long-standing limits.parse.

The lock file already contained `limits` (5.8.0) as a transitive entry, so the
lock change is just the new edge from flexmeasures to it; no resolved version
changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JEvWAj45zXaod5WjniF81D
Signed-off-by: F.N. Claessen <felix@seita.nl>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JEvWAj45zXaod5WjniF81D
Signed-off-by: F.N. Claessen <felix@seita.nl>
@read-the-docs-community

read-the-docs-community Bot commented Aug 7, 2026

Copy link
Copy Markdown

Comment thread documentation/changelog.rst Outdated
Flix6x added 2 commits August 13, 2026 19:29
Signed-off-by: F.N. Claessen <felix@seita.nl>

# Conflicts:
#	documentation/changelog.rst
Context:
- Review: no entry of its own; just append the PR reference to the rate limiting entry

Change:
- Dropped the separate Infrastructure / Support entry and referenced this PR from
  the entry of #2306, which introduced the rate limiting this dependency serves

Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening

Copy link
Copy Markdown
Member

Should this be in v1.0?

@Flix6x
Flix6x merged commit 9d66298 into main Aug 14, 2026
13 checks passed
@Flix6x
Flix6x deleted the fix/declare-rate-limiting-deps branch August 14, 2026 19:33
@Flix6x

Flix6x commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Should this be in v1.0?

Yes. But I think it's a technicality.

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