refactor(enterprise): derive settings from community base to reduce duplication #3121
Conversation
…uplication - Replace redundant configuration in `enterprise_core/settings.py` with `from ciso_assistant.settings import *`. - Extend `INSTALLED_APPS` and `REST_FRAMEWORK` permissions via list appending rather than re-declaration. - Configure `MODULE_PATHS`, `ROUTES`, and `MODULES` by updating the imported dictionaries. - Remove duplicate logging, database, authentication, and middleware logic. - Reduce enterprise settings file size (~90% reduction) and ensure future community updates propagate automatically.
📝 WalkthroughWalkthroughThe Django settings file for enterprise_core was restructured to inherit from a shared base configuration, with selective runtime overrides. License-related parameters and modular wiring declarations (serializers, routes, modules) were added to support enterprise licensing and extensibility features. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enterprise/backend/enterprise_core/settings.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T07:52:08.106Z
Learnt from: monsieurswag
Repo: intuitem/ciso-assistant-community PR: 3074
File: backend/ciso_assistant/settings.py:120-127
Timestamp: 2025-12-19T07:52:08.106Z
Learning: In Django-style settings files, SECRET_KEY should not be hard-coded in production. Use an environment variable (e.g., DJANGO_SECRET_KEY) for production and keep a dev-only fallback mechanism if needed (e.g., a local file like .django_secret_key). Ensure production always reads from the environment and avoid exposing keys in source control. If you generate a key, constrain the allowed characters to the explicit charset used by get_random_secret_key(): !#$%&()*+-0123456789=@^_abcdefghijklmnopqrstuvwxyz, and ensure there is no whitespace. This pattern should apply to all settings.py files across Django projects, not just this specific file.
Applied to files:
enterprise/backend/enterprise_core/settings.py
🧬 Code graph analysis (1)
enterprise/backend/enterprise_core/settings.py (1)
enterprise/backend/enterprise_core/views.py (2)
info(91-100)get(224-250)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build_enterprise_frontend
- GitHub Check: build_community_frontend
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
enterprise/backend/enterprise_core/settings.py (3)
25-31: LGTM - mutation-based extension is appropriate here.Appending to
INSTALLED_APPSandREST_FRAMEWORK["DEFAULT_PERMISSION_CLASSES"]is a clean approach for enterprise extensions. This correctly leverages the base configuration while adding enterprise-specific entries.Ensure the base
ciso_assistant.settingsdefinesINSTALLED_APPSandREST_FRAMEWORK["DEFAULT_PERMISSION_CLASSES"]as lists (not tuples), otherwise.append()will fail at runtime.
33-45: LGTM - clean modular wiring for enterprise features.The enterprise module registration pattern is well-structured, populating the base-defined dictionaries with enterprise-specific routes and modules.
56-60: LGTM - useful startup diagnostics.Logging the feature flags and module paths at startup provides good visibility into the enterprise configuration state.
| # Override default Log Level if not set in ENV (Community defaults to WARNING) | ||
| if "LOG_LEVEL" not in os.environ: | ||
| LOG_LEVEL = "INFO" | ||
| # Note: LOGGING dict was already configured in the base import. | ||
| # To strictly enforce INFO without an ENV var, you would need to update | ||
| # LOGGING['loggers']['']['level'] and re-run dictConfig here. | ||
| # However, it is cleaner to manage this via the .env file. |
There was a problem hiding this comment.
LOG_LEVEL assignment has no effect on actual logging.
As the comment notes, setting LOG_LEVEL here won't update the already-configured LOGGING dict from the base import. This creates a misleading variable that doesn't affect runtime behavior.
Either remove this block entirely (manage via .env as suggested), or actually reconfigure logging:
🔎 Option: Actually apply the override
# Override default Log Level if not set in ENV (Community defaults to WARNING)
if "LOG_LEVEL" not in os.environ:
LOG_LEVEL = "INFO"
- # Note: LOGGING dict was already configured in the base import.
- # To strictly enforce INFO without an ENV var, you would need to update
- # LOGGING['loggers']['']['level'] and re-run dictConfig here.
- # However, it is cleaner to manage this via the .env file.
+ LOGGING["loggers"][""]["level"] = LOG_LEVEL📝 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.
| # Override default Log Level if not set in ENV (Community defaults to WARNING) | |
| if "LOG_LEVEL" not in os.environ: | |
| LOG_LEVEL = "INFO" | |
| # Note: LOGGING dict was already configured in the base import. | |
| # To strictly enforce INFO without an ENV var, you would need to update | |
| # LOGGING['loggers']['']['level'] and re-run dictConfig here. | |
| # However, it is cleaner to manage this via the .env file. | |
| # Override default Log Level if not set in ENV (Community defaults to WARNING) | |
| if "LOG_LEVEL" not in os.environ: | |
| LOG_LEVEL = "INFO" | |
| LOGGING["loggers"][""]["level"] = LOG_LEVEL |
🤖 Prompt for AI Agents
In enterprise/backend/enterprise_core/settings.py around lines 9 to 15, the
assignment to LOG_LEVEL does not affect the already-configured LOGGING dict and
is therefore misleading; either remove this block entirely or implement a real
override by reading the desired level, updating LOGGING['loggers']['']['level']
(and any other relevant logger levels) to that value, then re-applying the
configuration via logging.config.dictConfig(LOGGING) (ensuring logging.config is
imported) so the runtime logging level actually changes.
| LICENSE_SEATS = int(os.environ.get("LICENSE_SEATS", 1)) | ||
| LICENSE_EXPIRATION = os.environ.get("LICENSE_EXPIRATION", "unset") |
There was a problem hiding this comment.
Add error handling for LICENSE_SEATS parsing.
If LICENSE_SEATS environment variable contains a non-integer value, int() will raise ValueError and crash the application on startup.
🔎 Proposed fix with graceful fallback
-LICENSE_SEATS = int(os.environ.get("LICENSE_SEATS", 1))
+try:
+ LICENSE_SEATS = int(os.environ.get("LICENSE_SEATS", 1))
+except ValueError:
+ logger.warning("Invalid LICENSE_SEATS value, defaulting to 1")
+ LICENSE_SEATS = 1
LICENSE_EXPIRATION = os.environ.get("LICENSE_EXPIRATION", "unset")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In enterprise/backend/enterprise_core/settings.py around lines 51 to 52, parsing
LICENSE_SEATS with int(os.environ.get(...)) can raise ValueError for non-integer
env values; wrap the conversion in a try/except (or use a helper that attempts
int and falls back) to catch ValueError and set LICENSE_SEATS to a safe default
(e.g., 1) when parsing fails, optionally validate the result (e.g., ensure >=1)
and emit a warning or logger message indicating the invalid environment value
and the fallback.
Refactoring this to preserve developer sanity, as manually syncing two nearly identical files was becoming a tedious exercise in frustration.
enterprise_core/settings.pywithfrom ciso_assistant.settings import *.INSTALLED_APPSandREST_FRAMEWORKpermissions via listappending rather than re-declaration.
MODULE_PATHS,ROUTES, andMODULESby updating theimported dictionaries.
logic.
future community updates propagate automatically.
Summary by CodeRabbit
Release Notes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.