Declare limits as a direct dependency - #2407
Merged
Merged
Conversation
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>
Documentation build overview
54 files changed ·
|
Flix6x
commented
Aug 8, 2026
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>
Member
|
Should this be in v1.0? |
Member
Author
Yes. But I think it's a technicality. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
flexmeasures/utils/validation_utils.pydoesimport limits(to validate rate limits vialimits.parse), butlimitswas not declared inpyproject.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 thenNo 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-Limiteris declared. PR Rate limiting plans for the API, and for scheduling and forecasting triggers in particular #2306 added"Flask-Limiter[redis]>=4.0"topyproject.tomlitself (git log -S 'Flask-Limiter' -- pyproject.tomlpoints straight atfecf13a96). So the second error was a stale-venv symptom, not a packaging gap — those venvs simply had not been re-synced after the merge.limitsis genuinely undeclared, but it still installs. Flask-Limiter 4.1.1 requireslimits>=3.13, so a fresh install from the declared dependencies does pulllimitsin transitively. I verified this: a clean venv installed frompyproject.tomlon unmodifiedmaingetslimits5.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
limitsdependency,import flexmeasuresbreaks with no signal from our own metadata — and nothing in CI would catch it, since CI installs withuv sync --frozen, which resolves purely fromuv.lock, wherelimitsis pinned regardless of who asked for it. The declared-dependency path is never exercised.What changed
pyproject.toml: added"limits>=3.13", next toFlask-Limiter. The floor is Flask-Limiter's own requirement — we only uselimits.parse, which is long-standing, so there is no reason to demand more.uv.lock: the newflexmeasures -> limitsedge and its specifier. No resolved version changed —limits5.8.0 was already locked as a transitive dependency, so the diff is two lines. (I hand-applied it rather than committing a fulluv lockregeneration, which would have rewritten ~1500 lines of environment-marker serialization with zero version changes.uv lock --checkpasses on the result, as doesuv 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
limitsis added.Flask-Limiterneeds no change, and adding it would be a no-op.Verification
The meaningful test here is a lock-free install, since a passing
uv syncproves nothing. In a fresh Python 3.12 venv, installed from the declared dependencies only (uv pip install ., nouv.lock):validate_rate_limit("500 per minute")and importingflexmeasures.api.common.rate_limitingboth 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 frompyproject.tomlwithout the lock backtracksnumbaall the way to 0.47.0 (viadarts), which then tries to buildllvmlite0.36.0 and fails on Python 3.12. I worked around it locally withnumba>=0.61to 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 auv sync --frozenCI never sees. Probably worth a separate issue and anumbafloor.