-
Notifications
You must be signed in to change notification settings - Fork 22
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
Website backend: Changed debug to .env value #4309
base: staging
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a modification to the Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## staging #4309 +/- ##
===========================================
- Coverage 11.22% 11.20% -0.03%
===========================================
Files 155 156 +1
Lines 17922 18195 +273
Branches 388 388
===========================================
+ Hits 2012 2038 +26
- Misses 15908 16155 +247
Partials 2 2 |
website changes in this PR available for preview here |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/website/core/settings.py
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-push-deploy-website
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
src/website/core/settings.py (2)
Line range hint
119-120
: Well-structured security configuration!The cookie security settings are properly tied to the DEBUG setting, ensuring secure defaults in production while maintaining development convenience.
Line range hint
267-399
: Excellent logging configuration!The logging setup intelligently adjusts verbosity based on the DEBUG setting across all application loggers. This ensures detailed logs during development while maintaining efficient logging in production.
@@ -50,8 +50,7 @@ def require_env_var(env_var: str) -> str: | |||
# Core Settings | |||
# --------------------------------------------------------- | |||
SECRET_KEY = require_env_var('SECRET_KEY') | |||
# DEBUG = get_env_bool('DEBUG', default=False) | |||
DEBUG = True | |||
DEBUG = get_env_bool('DEBUG', default=False) |
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.
💡 Codebase verification
Update .env.sample documentation for DEBUG setting
The .env.sample
file currently sets DEBUG=True
without any explanation. While this is convenient for development, it should include a clear warning about production deployments. Please add a comment:
# Development only! Set DEBUG=False in production for security
DEBUG=True
🔗 Analysis chain
Excellent security improvement!
Moving from hardcoded DEBUG=True
to environment variable control with secure defaults is a significant improvement. This change follows Django best practices and the 12-factor app methodology.
Please ensure the .env.example
file is updated to include:
DEBUG=False # Set to True for local development
Run this script to verify the documentation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if DEBUG is documented in .env.example
fd -t f ".env.example" -x grep -l "DEBUG="
Length of output: 40
Script:
#!/bin/bash
# Search for any environment-related files and documentation
echo "=== Environment Files ==="
fd -t f "^\.env" || true
echo -e "\n=== Documentation Files ==="
fd -t f "^README|^CONTRIBUTING|^DEVELOPMENT|^ENV" -i || true
echo -e "\n=== Checking Documentation Content ==="
fd -t f "^README|^CONTRIBUTING|^DEVELOPMENT|^ENV" -i -x grep -l "DEBUG" || true
Length of output: 2183
Script:
#!/bin/bash
# Search specifically in website directory with hidden files
echo "=== Website Environment Files ==="
fd -H -t f "^\.env" src/website || true
echo -e "\n=== Website Config Files ==="
fd -t f . src/website/config || true
echo -e "\n=== Website README Content ==="
cat src/website/README.md || true
Length of output: 465
Script:
#!/bin/bash
# Check content of .env.sample file
echo "=== .env.sample Content ==="
cat src/website/.env.sample | grep -i "debug" -A 1 -B 1 || true
Length of output: 228
Description
[Provide a brief description of the changes made in this PR]
Related Issues
Changes Made
Testing
Affected Services
Endpoints Ready for Testing
API Documentation Updated?
Additional Notes
[Add any additional notes or comments here]
Summary by CodeRabbit
False
when not explicitly specified