Skip to content

feat: wire in rate limiter with HTTPException handler and env var disable#277

Closed
shivv23 wants to merge 1 commit intoc2siorg:mainfrom
shivv23:pr1-rate-limiting
Closed

feat: wire in rate limiter with HTTPException handler and env var disable#277
shivv23 wants to merge 1 commit intoc2siorg:mainfrom
shivv23:pr1-rate-limiting

Conversation

@shivv23
Copy link
Copy Markdown
Contributor

@shivv23 shivv23 commented Apr 3, 2026

Description:
Wire rate limiter into main.py with HTTPException handler
Add rate_limit_enabled config option to disable via env var

Changes:
app/config.py: Add rate_limit_enabled: bool = True
app/main.py: Conditionally add rate limit middleware, add HTTPException handler for 429 responses
Checklist:

✅ Rate limit enforced on training endpoints
✅ 429 returns JSON: {"success": false, "message": "...", "data": null}
✅ Can disable via RATE_LIMIT_ENABLED=false

@shivv23 shivv23 force-pushed the pr1-rate-limiting branch 4 times, most recently from cf8a740 to f9986e7 Compare April 6, 2026 20:44
@shivv23 shivv23 force-pushed the pr1-rate-limiting branch from f9986e7 to dd7d90d Compare April 11, 2026 18:54
@ivantha
Copy link
Copy Markdown
Member

ivantha commented Apr 13, 2026

The PR imports RateLimitMiddleware and get_training_limiter from app.rate_limiter, but the tensormap-backend/app/rate_limiter.py module is not included in the diff — it appears to be missing from the branch entirely. The application will fail to start with an ImportError. Please add the rate_limiter.py implementation (and any new dependencies in pyproject.toml/uv.lock) to the branch, then this can be re-reviewed.

@shivv23
Copy link
Copy Markdown
Contributor Author

shivv23 commented Apr 14, 2026

You're right @ivantha - the rate_limiter.py implementation is missing from this PR. That's because the implementation was added in PR #288 (which is now mergeable). This PR #277 just wires the rate limiter into main.py, but it depends on #288 being merged first.I'll close this PR and re-create it after #288 is merged so it has the full implementation.

@shivv23 shivv23 closed this Apr 14, 2026
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