Conversation
CORS and per-IP rate limiting (slowapi) are no longer needed. This removes the CORSMiddleware setup, slowapi dependency, related config functions, environment variables, constants, tests, and documentation references. Market-book subscription limits remain unchanged. https://claude.ai/code/session_01SC3baPqBUyQjTmKkPctbUp
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review Summary: Clean, well-scoped removal. The architectural decision to delegate rate limiting and CORS to infrastructure is sound. The application code, constants, config, and test cleanup is thorough. However, there are stale documentation references and a security documentation gap that should be addressed before merge. See inline comments for details.
Detailed findingsDocumentation: README.md not updated (4 stale references) README.md was not touched by this PR but still contains references to the removed features:
Documentation: docs/index.md line 10 Overview paragraph still reads: adds optional authentication, rate limiting, and response formatting suitable for analytics workflows. Remove rate limiting reference. Security: docs/api/rest-api.md security checklist The security checklist previously included rate limiting as a minimum security posture item. Consider adding: Configure rate limiting at the reverse proxy or API gateway level -- to make it clear this responsibility has moved to infrastructure rather than being silently dropped. |
Tests for these endpoints have not been completed, so label them as experimental in both the OpenAPI summaries/descriptions and the REST API documentation. https://claude.ai/code/session_01SC3baPqBUyQjTmKkPctbUp
Address review comments on PR #21: - README.md: remove rate limiting from description, Mermaid diagram, and feature list; remove configurable CORS from feature list - docs/index.md: remove rate limiting from overview paragraph - docs/api/rest-api.md: add security checklist note to configure rate limiting at the reverse proxy or API gateway level https://claude.ai/code/session_01SC3baPqBUyQjTmKkPctbUp
Address review comments on PR #21: - README.md: remove rate limiting from description, Mermaid diagram, and feature list; remove configurable CORS from feature list - docs/index.md: remove rate limiting from overview paragraph - docs/api/rest-api.md: add security checklist note to configure rate limiting at the reverse proxy or API gateway level https://claude.ai/code/session_01SC3baPqBUyQjTmKkPctbUp
This PR removes the rate limiting and CORS middleware functionality from the MT5 API, simplifying the middleware stack and reducing external dependencies.
Summary
The rate limiting (via
slowapi) and CORS middleware have been removed from the application. These features are better handled by external infrastructure (reverse proxies, API gateways) rather than at the application level.Key Changes
Removed rate limiting: Deleted
slowapidependency and all rate limit configuration_build_default_rate_limit()function from middleware_rate_limit_exceeded_handler()exception handlerMT5API_RATE_LIMITenvironment variable and related config functionsRemoved CORS middleware: Deleted Starlette CORS middleware setup
_get_cors_origins()function from main.pyMT5API_CORS_ORIGINSenvironment variable and related config functionsSimplified middleware stack:
add_middleware()now only registers error handling and logging middlewareUpdated documentation: Removed references to rate limiting and CORS configuration from deployment and API docs
Updated dependencies: Removed
slowapi >= 0.1.9from pyproject.tomlImplementation Details
The middleware stack is now simplified to only handle:
Rate limiting and CORS should be configured at the infrastructure level (e.g., nginx, API Gateway, load balancer) for better performance and flexibility.
https://claude.ai/code/session_01SC3baPqBUyQjTmKkPctbUp