-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add thread-safe token bucket rate limiter utility #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| """ | ||
| Thread-safe rate limiter using the token bucket algorithm. | ||
|
|
||
| Useful for throttling API calls to AWS services or other external endpoints | ||
| to stay within service quotas. | ||
| """ | ||
|
|
||
| import threading | ||
| import time | ||
|
|
||
|
|
||
| class RateLimiter: | ||
| """ | ||
| A token bucket rate limiter. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| rate : float | ||
| Number of tokens added per second. | ||
| burst : int | ||
| Maximum number of tokens the bucket can hold. | ||
|
|
||
| Examples | ||
| -------- | ||
| >>> limiter = RateLimiter(rate=10, burst=10) | ||
| >>> limiter.acquire() # blocks until a token is available | ||
| >>> limiter.try_acquire() # returns True/False without blocking | ||
| """ | ||
|
|
||
| def __init__(self, rate: float, burst: int): | ||
| if rate <= 0: | ||
| raise ValueError("rate must be positive") | ||
| if burst <= 0: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nit: |
||
| raise ValueError("burst must be a positive integer") | ||
|
|
||
| self._rate = float(rate) | ||
| self._burst = burst | ||
| self._tokens = float(burst) | ||
| self._last_refill = time.monotonic() | ||
| self._lock = threading.Lock() | ||
|
|
||
| def _refill(self): | ||
| now = time.monotonic() | ||
| elapsed = now - self._last_refill | ||
| self._tokens = min(self._burst, self._tokens + elapsed * self._rate) | ||
| self._last_refill = now | ||
|
|
||
| def try_acquire(self, tokens: int = 1) -> bool: | ||
| """ | ||
| Try to consume tokens without blocking. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| tokens : int | ||
| Number of tokens to consume. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if tokens were acquired, False otherwise. | ||
| """ | ||
| with self._lock: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Critical: Negative Neither Fix: Add validation at the top of both methods: if tokens <= 0:
raise ValueError("tokens must be positive") |
||
| self._refill() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Major: If a caller requests more tokens than the bucket can ever hold, this is almost certainly a programming error — but the caller gets no feedback, just an eternal Fix: Validate in both methods: if tokens > self._burst:
raise ValueError(f"tokens ({tokens}) exceeds burst capacity ({self._burst})") |
||
| if self._tokens >= tokens: | ||
| self._tokens -= tokens | ||
| return True | ||
| return False | ||
|
|
||
| def acquire(self, tokens: int = 1, timeout: float = None) -> bool: | ||
| """ | ||
| Block until tokens are available or timeout is reached. | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Minor: The rest of the |
||
| Parameters | ||
| ---------- | ||
| tokens : int | ||
| Number of tokens to consume. | ||
| timeout : float, optional | ||
| Maximum seconds to wait. None means wait indefinitely. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if tokens were acquired, False if timed out. | ||
| """ | ||
| deadline = None if timeout is None else time.monotonic() + timeout | ||
|
|
||
| while True: | ||
| with self._lock: | ||
| self._refill() | ||
| if self._tokens >= tokens: | ||
| self._tokens -= tokens | ||
| return True | ||
| deficit = tokens - self._tokens | ||
| wait_time = deficit / self._rate | ||
|
|
||
| if deadline is not None: | ||
| remaining = deadline - time.monotonic() | ||
| if remaining <= 0: | ||
| return False | ||
| wait_time = min(wait_time, remaining) | ||
|
|
||
| time.sleep(wait_time) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| """Tests for the rate limiter utility.""" | ||
|
|
||
| import threading | ||
| import time | ||
| from unittest import TestCase | ||
|
|
||
| from samcli.lib.utils.rate_limiter import RateLimiter | ||
|
|
||
|
|
||
| class TestRateLimiter(TestCase): | ||
| def test_try_acquire_succeeds_when_tokens_available(self): | ||
| limiter = RateLimiter(rate=10, burst=5) | ||
| self.assertTrue(limiter.try_acquire()) | ||
|
|
||
| def test_try_acquire_fails_when_exhausted(self): | ||
| limiter = RateLimiter(rate=10, burst=2) | ||
| self.assertTrue(limiter.try_acquire()) | ||
| self.assertTrue(limiter.try_acquire()) | ||
| self.assertFalse(limiter.try_acquire()) | ||
|
|
||
| def test_tokens_refill_over_time(self): | ||
| limiter = RateLimiter(rate=100, burst=1) | ||
| self.assertTrue(limiter.try_acquire()) | ||
| self.assertFalse(limiter.try_acquire()) | ||
| time.sleep(0.02) # wait for refill | ||
| self.assertTrue(limiter.try_acquire()) | ||
|
|
||
| def test_acquire_with_timeout_returns_false_on_expiry(self): | ||
| limiter = RateLimiter(rate=1, burst=1) | ||
| limiter.try_acquire() # drain | ||
| self.assertFalse(limiter.acquire(timeout=0.01)) | ||
|
|
||
| def test_acquire_blocks_until_available(self): | ||
| limiter = RateLimiter(rate=100, burst=1) | ||
| limiter.try_acquire() # drain | ||
| start = time.monotonic() | ||
| self.assertTrue(limiter.acquire(timeout=1.0)) | ||
| elapsed = time.monotonic() - start | ||
| self.assertLess(elapsed, 0.5) | ||
|
|
||
| def test_invalid_rate_raises(self): | ||
| with self.assertRaises(ValueError): | ||
| RateLimiter(rate=0, burst=1) | ||
|
|
||
| def test_invalid_burst_raises(self): | ||
| with self.assertRaises(ValueError): | ||
| RateLimiter(rate=1, burst=0) | ||
|
|
||
| def test_thread_safety(self): | ||
| limiter = RateLimiter(rate=1000, burst=100) | ||
| results = [] | ||
|
|
||
| def worker(): | ||
| acquired = sum(1 for _ in range(20) if limiter.try_acquire()) | ||
| results.append(acquired) | ||
|
|
||
| threads = [threading.Thread(target=worker) for _ in range(10)] | ||
| for t in threads: | ||
| t.start() | ||
| for t in threads: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Major: Thread-safety test assertion is flawed — will produce false negatives. With Fix: Use a near-zero refill rate so the bucket doesn't meaningfully refill during the test: limiter = RateLimiter(rate=0.001, burst=100)This makes the assertion reliable while still testing thread safety of concurrent |
||
| t.join() | ||
|
|
||
| # Total acquired should not exceed burst (100) | ||
| self.assertLessEqual(sum(results), 100) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Minor: Missing test coverage for edge cases. Consider adding tests for:
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Minor: Missing module-level logger.
Every other non-trivial utility in
samcli/lib/utils/definesLOG = logging.getLogger(__name__). Theacquire()method's busy-wait loop is a great candidate forLOG.debug()calls to aid diagnostics (e.g., logging when waiting for token refill).