fix: align ee settings to include huey #1448
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enterprise/backend/enterprise_core/settings.py(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: Analyze (python)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: migrations-check (3.12)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
🔇 Additional comments (2)
enterprise/backend/enterprise_core/settings.py (2)
116-116: Add logging for email debug mode.For consistency with other debug settings (e.g., line 118 logs DEBUG mode), consider logging the MAIL_DEBUG state.
MAIL_DEBUG = os.environ.get("MAIL_DEBUG", "False") == "True" +logger.info("MAIL DEBUG mode: %s", MAIL_DEBUG)Also, verify that this doesn't conflict with the rescue mail configuration:
Also applies to: 434-437
✅ Verification successful
MAIL_DEBUG setting is compatible with rescue mail configuration
The debug setting intentionally overrides both primary and rescue email configurations when enabled, which is the expected behavior for development environments. The default False value ensures production email delivery (including rescue mail) works normally.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential email configuration conflicts rg "EMAIL_.*RESCUE" --type pyLength of output: 2939
161-161: Verify Huey package dependencies.The addition of Huey requires the package to be listed in requirements.txt and compatibility with the Django version.
✅ Verification successful
Dependencies are properly configured ✓
The required packages are correctly specified in the backend's pyproject.toml:
- huey = "^2.5.2"
- django = "5.1.5"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if huey is in requirements grep -r "huey" . --include="*requirements*.txt" # Check Django version compatibility grep -r "Django==" . --include="*requirements*.txt"Length of output: 99
Script:
#!/bin/bash # Find all Python dependency files echo "=== Searching for dependency files ===" fd -e txt -e in "requirements|reqs|deps" . fd "setup.py|pyproject.toml" . echo -e "\n=== Checking content of found files ===" # Use case-insensitive search and more flexible patterns fd -e txt -e in "requirements|reqs|deps" . -x cat {} fd "setup.py|pyproject.toml" . -x cat {}Length of output: 1982
| ## Huey settings | ||
| HUEY_FILE_PATH = os.environ.get("HUEY_FILE_PATH", BASE_DIR / "db" / "huey.db") | ||
|
|
||
| HUEY = { | ||
| "huey_class": "huey.SqliteHuey", | ||
| "name": "ciso_assistant", | ||
| "filename": HUEY_FILE_PATH, | ||
| "results": True, # would be interesting for debug | ||
| "immediate": False, # set to False to run in "live" mode regardless of DEBUG, otherwise it will follow | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance Huey configuration robustness.
Several improvements needed for the Huey configuration:
- The comment on line 447 is incomplete
- Missing directory existence check for HUEY_FILE_PATH
- Consider documenting SQLite limitations for high task volumes
## Huey settings
HUEY_FILE_PATH = os.environ.get("HUEY_FILE_PATH", BASE_DIR / "db" / "huey.db")
+# Ensure directory exists
+os.makedirs(os.path.dirname(HUEY_FILE_PATH), exist_ok=True)
HUEY = {
"huey_class": "huey.SqliteHuey",
"name": "ciso_assistant",
"filename": HUEY_FILE_PATH,
"results": True, # would be interesting for debug
- "immediate": False, # set to False to run in "live" mode regardless of DEBUG, otherwise it will follow
+ "immediate": False, # Set to False to run in "live" mode regardless of DEBUG setting
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Huey settings | |
| HUEY_FILE_PATH = os.environ.get("HUEY_FILE_PATH", BASE_DIR / "db" / "huey.db") | |
| HUEY = { | |
| "huey_class": "huey.SqliteHuey", | |
| "name": "ciso_assistant", | |
| "filename": HUEY_FILE_PATH, | |
| "results": True, # would be interesting for debug | |
| "immediate": False, # set to False to run in "live" mode regardless of DEBUG, otherwise it will follow | |
| } | |
| ## Huey settings | |
| HUEY_FILE_PATH = os.environ.get("HUEY_FILE_PATH", BASE_DIR / "db" / "huey.db") | |
| # Ensure directory exists | |
| os.makedirs(os.path.dirname(HUEY_FILE_PATH), exist_ok=True) | |
| HUEY = { | |
| "huey_class": "huey.SqliteHuey", | |
| "name": "ciso_assistant", | |
| "filename": HUEY_FILE_PATH, | |
| "results": True, # would be interesting for debug | |
| "immediate": False, # Set to False to run in "live" mode regardless of DEBUG setting | |
| } |
| @@ -339,6 +341,7 @@ def set_ciso_assistant_url(_, __, event_dict): | |||
| "PASSWORD": os.environ["POSTGRES_PASSWORD"], | |||
| "HOST": os.environ["DB_HOST"], | |||
| "PORT": os.environ.get("DB_PORT", "5432"), | |||
| "CONN_MAX_AGE": os.environ.get("CONN_MAX_AGE", 300), | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Convert CONN_MAX_AGE to integer and document load balancer considerations.
The CONN_MAX_AGE should be converted to an integer, and we should document potential issues with load balancers.
- "CONN_MAX_AGE": os.environ.get("CONN_MAX_AGE", 300),
+ "CONN_MAX_AGE": int(os.environ.get("CONN_MAX_AGE", "300")),Consider documenting that if you're using a load balancer, CONN_MAX_AGE should be less than the load balancer's connection timeout to prevent connection reset issues.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "CONN_MAX_AGE": os.environ.get("CONN_MAX_AGE", 300), | |
| "CONN_MAX_AGE": int(os.environ.get("CONN_MAX_AGE", "300")), |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
enterprise/backend/enterprise_core/settings.py (3)
116-116: Consider making the debug email address configurable.While the mail debug configuration is good, the hardcoded email address
[email protected]should be made configurable through environment variables to maintain consistency with other email settings in the file.if MAIL_DEBUG: EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" - DEFAULT_FROM_EMAIL = "[email protected]" + DEFAULT_FROM_EMAIL = os.environ.get("DEBUG_FROM_EMAIL", "[email protected]")Also applies to: 441-444
313-322: Consider alphabetizing language entries for better maintainability.The language entries should be alphabetically ordered for easier maintenance and readability. Also, consider adding documentation about the language selection criteria and a note about other languages that might be added in the future.
LANGUAGES = [ + ("ar", "Arabic"), + ("cs", "Czech"), ("de", "German"), ("en", "English"), ("es", "Spanish"), ("fr", "French"), + ("hi", "Hindi"), + ("id", "Indonesian"), ("it", "Italian"), ("nl", "Dutch"), ("pl", "Polish"), ("pt", "Portuguese"), + ("ro", "Romanian"), + ("sv", "Swedish"), + ("ur", "Urdu"), ]
446-455: Consider aligning Huey database backend with main database.When using PostgreSQL as the main database, consider using PostgreSQL for Huey as well to maintain consistency and avoid potential issues with SQLite under high load. This would require:
- Using
huey.PriorityRedisHueyor a custom PostgreSQL-based backend- Ensuring proper connection pooling
- Handling connection timeouts consistently with the main database
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enterprise/backend/enterprise_core/settings.py(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
enterprise/backend/enterprise_core/settings.py (2)
351-351: Convert CONN_MAX_AGE to integer and document load balancer considerations.The CONN_MAX_AGE should be converted to an integer, and we should document potential issues with load balancers.
446-455: Enhance Huey configuration robustness.Several improvements needed for the Huey configuration:
- The comment on line 447 is incomplete
- Missing directory existence check for HUEY_FILE_PATH
- Consider documenting SQLite limitations for high task volumes
Summary by CodeRabbit