RateLimit admission control dependency#356
Conversation
Adds `RateLimit(limit, per=timedelta)` — caps how many times a task (or a per-parameter scope) can execute within a sliding window. Uses a single Redis sorted set per scope with a Lua script that atomically prunes old entries, counts remaining, and either records the execution or computes `retry_after` from the oldest entry. By default, excess tasks are rescheduled to exactly when a slot opens. `drop=True` quietly drops them instead (like Cooldown). Works both as a default parameter and as `Annotated` metadata for per-parameter scoping — same patterns as Cooldown and Debounce. Closes #161. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Documentation build overview
Show files changed (2 files in total): 📝 2 modified | ➕ 0 added | ➖ 0 deleted
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e7e7b824e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| script = redis.register_script(_RATELIMIT_LUA) | ||
| result: list[int] = await script( | ||
| keys=[ratelimit_key], | ||
| args=[execution.key, now_ms, window_ms, self.limit, ttl_ms], |
There was a problem hiding this comment.
Count each execution, not each task key
Using execution.key as the sorted-set member means repeated runs of the same key overwrite a single entry instead of increasing the window count, so the limiter can be bypassed whenever keys are reused (for example, Perpetual.on_complete reschedules with the same key in src/docket/dependencies/_perpetual.py). In those cases ZCARD stays at 1 and limits like RateLimit(10, ...) never trigger as intended.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #356 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 106 108 +2
Lines 3106 3159 +53
Branches 26 26
=========================================
+ Hits 3106 3159 +53
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Using `execution.key` as the sorted set member meant Perpetual tasks
(which reuse the same key via `replace()`) only ever had one entry —
ZADD overwrites the score instead of adding a new member, so ZCARD
stays at 1 and the rate limit never fires.
The member is now `{execution.key}:{now_ms}`, so each execution
attempt gets its own entry. Also cleans up phantom slots in
`__aexit__` when a later dependency (like ConcurrencyLimit) blocks —
the task never ran but would otherwise consume a rate-limit slot.
Caught during PR #356 review.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds
RateLimit(limit, per=timedelta)— caps how many times a task (or aper-parameter scope) can execute within a sliding window. Uses a single
Redis sorted set per scope with a Lua script that atomically prunes old
entries, counts remaining, and either records the execution or computes
retry_afterfrom the oldest entry.By default, excess tasks are rescheduled to exactly when a slot opens.
drop=Truequietly drops them instead (like Cooldown).Works both as a default parameter and as
Annotatedmetadata forper-parameter scoping — same patterns as Cooldown and Debounce.
Closes #161.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com