Skip to content

Conversation

priyanshu-panwar
Copy link
Owner

@priyanshu-panwar priyanshu-panwar commented Mar 29, 2025

In-memory rate limiter
TODO: REDIS one to be implemented later

@priyanshu-panwar priyanshu-panwar linked an issue Mar 29, 2025 that may be closed by this pull request
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements an in-memory rate limiter for FastAPI along with tests and documentation updates, laying the groundwork for a future REDIS-based implementation.

  • Adds a rate limiter decorator in fastapi_utilities/rate_limiter/rate_limiter.py
  • Introduces comprehensive tests for multiple rate limiting scenarios in tests/test_rate_limiter.py
  • Updates documentation in README.md and docs/index.md to describe the new rate limiter

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_rate_limiter.py Added tests covering various rate limiting scenarios
fastapi_utilities/rate_limiter/rate_limiter.py Implements the in-memory rate limiter decorator
fastapi_utilities/rate_limiter/init.py Exposes the rate limiter decorator
fastapi_utilities/init.py Updates package initialization to include the rate limiter
docs/index.md Updates documentation and table of contents for the rate limiter
app.py Provides sample usage of the rate limiter
README.md Updates documentation with details on rate limiter usage
Files not reviewed (1)
  • .vscode/settings.json: Language not supported

Comment on lines +7 to +8
request_logs: Dict[str, Tuple[float, int]] = {}

Copy link

Copilot AI Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation of the global 'request_logs' dictionary is not thread-safe and may lead to race conditions in concurrent environments. Consider protecting its access with an async lock or another concurrency control mechanism.

Suggested change
request_logs: Dict[str, Tuple[float, int]] = {}
request_logs: Dict[str, Tuple[float, int]] = {}
request_logs_lock = asyncio.Lock()

Copilot uses AI. Check for mistakes.

Copy link

codecov bot commented Mar 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.89%. Comparing base (7058387) to head (6c66526).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
+ Coverage   73.00%   75.89%   +2.89%     
==========================================
  Files          12       14       +2     
  Lines         200      224      +24     
==========================================
+ Hits          146      170      +24     
  Misses         54       54              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@apoplexi24
Copy link
Contributor

I noticed that while this approach works well with uvicorn, it may encounter issues when using gunicorn with multiple worker processes. Since gunicorn spawns separate processes for each worker, global variables won't sync between instances, which can lead to inconsistent rate-limiting behavior. We could instead use redis or if needed to be more optimized and for a single instance, we can use sqlite.

Here is a flowchart that I drew for my blog to understand how out of sync gunicorn workers are!
gunicorn-flowchart

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.

New Feature: FastAPI Rate Limiter

2 participants