chore: configuration and infrastructure polish#123
Conversation
- Add Redis persistence volume - Extract REDIS_HOST, REDIS_PORT, and CORS_ORIGINS to environment variables
|
@abhiyanpa is attempting to deploy a commit to the lamsta Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Code Review
This pull request externalizes configuration for Redis and CORS by moving hardcoded values into environment variables and the settings module. It also adds data persistence for Redis in the Docker Compose configuration. Feedback was provided regarding the robustness of environment variable parsing in settings.py, specifically suggesting safer handling for the REDIS_PORT integer conversion and more flexible parsing of the CORS_ORIGINS comma-separated list to handle whitespace and empty values.
|
|
||
| # Redis configuration | ||
| REDIS_HOST = os.getenv("REDIS_HOST", "redis") | ||
| REDIS_PORT = int(os.getenv("REDIS_PORT", "6379")) |
There was a problem hiding this comment.
The int() conversion will raise a ValueError if the REDIS_PORT environment variable is set to an empty string in the .env file. It is safer to use a pattern that handles empty strings by falling back to the default value before conversion.
| REDIS_PORT = int(os.getenv("REDIS_PORT", "6379")) | |
| REDIS_PORT = int(os.getenv("REDIS_PORT") or 6379) |
| REDIS_PORT = int(os.getenv("REDIS_PORT", "6379")) | ||
|
|
||
| # CORS configuration | ||
| CORS_ORIGINS = os.getenv("CORS_ORIGINS", "https://devb.io,https://beta.devb.io,http://localhost:3000,http://localhost:8080").split(",") |
There was a problem hiding this comment.
Parsing CORS_ORIGINS by simply splitting on commas is fragile. It does not handle potential whitespace between origins (e.g., "origin1, origin2") and results in a list containing an empty string [''] if the environment variable is explicitly set to empty. Using a list comprehension with strip() and a truthiness check makes the configuration more robust.
| CORS_ORIGINS = os.getenv("CORS_ORIGINS", "https://devb.io,https://beta.devb.io,http://localhost:3000,http://localhost:8080").split(",") | |
| CORS_ORIGINS = [origin.strip() for origin in os.getenv("CORS_ORIGINS", "https://devb.io,https://beta.devb.io,http://localhost:3000,http://localhost:8080").split(",") if origin.strip()] |
Uh oh!
There was an error while loading. Please reload this page.