Merge pull request #16412 from edx/pwnage101/settings_validation_refactor_PLAT-1776

move settings validation out of startup.py

cms/envs/common.py

@@ -925,6 +925,9 @@ INSTALLED_APPS = [
     'djcelery',
     'method_override',
 
+    # Common Initialization
+    'openedx.core.djangoapps.common_initialization.apps.CommonInitializationConfig',
+
     # Common views
     'openedx.core.djangoapps.common_views',
 

cms/startup.py

@@ -15,7 +15,6 @@ from openedx.core.lib.django_startup import autostartup
 settings.INSTALLED_APPS  # pylint: disable=pointless-statement
 
 from openedx.core.lib.xblock_utils import xblock_local_resource_url
-from startup_configurations.validate_config import validate_cms_config
 
 
 def run():
@@ -40,9 +39,6 @@ def run():
     xmodule.x_module.descriptor_global_handler_url = cms.lib.xblock.runtime.handler_url
     xmodule.x_module.descriptor_global_local_resource_url = xblock_local_resource_url
 
-    # validate configurations on startup
-    validate_cms_config(settings)
-
 
 def add_mimetypes():
     """

common/djangoapps/startup_configurations/validate_config.py

@@ -1,42 +0,0 @@
-"""
-Common Functions to Validate Configurations
-"""
-
-
-def validate_lms_config(settings):
-    """
-    Validates configurations for lms and raise ValueError if not valid
-    """
-    validate_common_config(settings)
-
-    # validate feature based configurations
-    validate_marketing_site_config(settings)
-
-
-def validate_cms_config(settings):
-    """
-    Validates configurations for lms and raise ValueError if not valid
-    """
-    validate_common_config(settings)
-
-    # validate feature based configurations
-    validate_marketing_site_config(settings)
-
-
-def validate_common_config(settings):
-    """
-    Validates configurations common for all apps
-    """
-    if not getattr(settings, 'LMS_ROOT_URL', None):
-        raise ValueError("'LMS_ROOT_URL' is not defined.")
-
-
-def validate_marketing_site_config(settings):
-    """
-    Validates 'marketing site' related configurations
-    """
-    if settings.FEATURES.get('ENABLE_MKTG_SITE'):
-        if not hasattr(settings, 'MKTG_URLS'):
-            raise ValueError("'ENABLE_MKTG_SITE' is True, but 'MKTG_URLS' is not defined.")
-        if not settings.MKTG_URLS.get('ROOT'):
-            raise ValueError("There is no 'ROOT' defined in 'MKTG_URLS'.")

lms/envs/common.py

@@ -2034,7 +2034,10 @@ INSTALLED_APPS = [
     'django.contrib.staticfiles',
     'djcelery',
 
-    # Initialization
+    # Common Initialization
+    'openedx.core.djangoapps.common_initialization.apps.CommonInitializationConfig',
+
+    # LMS-specific Initialization
     'lms_initialization.apps.LMSInitializationConfig',
 
     # Common views

lms/startup.py

@@ -18,8 +18,6 @@ from openedx.core.djangoapps.monkey_patch import django_db_models_options
 import xmodule.x_module
 import lms_xblock.runtime
 
-from startup_configurations.validate_config import validate_lms_config
-
 log = logging.getLogger(__name__)
 
 
@@ -45,9 +43,6 @@ def run():
     xmodule.x_module.descriptor_global_handler_url = lms_xblock.runtime.handler_url
     xmodule.x_module.descriptor_global_local_resource_url = lms_xblock.runtime.local_resource_url
 
-    # validate configurations on startup
-    validate_lms_config(settings)
-
 
 def add_mimetypes():
     """

openedx/core/djangoapps/common_initialization/apps.py

@@ -0,0 +1,14 @@
+"""
+Common initialization app for the LMS and CMS
+"""
+
+from django.apps import AppConfig
+
+
+class CommonInitializationConfig(AppConfig):
+    name = 'openedx.core.djangoapps.common_initialization'
+    verbose_name = 'Common Initialization'
+
+    def ready(self):
+        # Common settings validations for the LMS and CMS.
+        from . import checks

openedx/core/djangoapps/common_initialization/checks.py

@@ -0,0 +1,49 @@
+"""
+Common settings validations for the LMS and CMS.
+
+Only populate this module with general settings validators which do not fit in
+other, more specific djangoapps.  Usually, settings which are widely used
+across the entire LMS or CMS can be validated here.
+"""
+from django.conf import settings
+from django.core.checks import Error, Tags, register
+
+
+@register(Tags.compatibility)
+def validate_lms_root_url_setting(app_configs, **kwargs):
+    """
+    Validates the LMS_ROOT_URL setting.
+    """
+    errors = []
+    if not getattr(settings, 'LMS_ROOT_URL', None):
+        errors.append(
+            Error(
+                'LMS_ROOT_URL is not defined.',
+                id='common.djangoapps.common_initialization.E001',
+            )
+        )
+    return errors
+
+
+@register(Tags.compatibility)
+def validate_marketing_site_setting(app_configs, **kwargs):
+    """
+    Validates marketing site related settings.
+    """
+    errors = []
+    if settings.FEATURES.get('ENABLE_MKTG_SITE'):
+        if not hasattr(settings, 'MKTG_URLS'):
+            errors.append(
+                Error(
+                    'ENABLE_MKTG_SITE is True, but MKTG_URLS is not defined.',
+                    id='common.djangoapps.common_initialization.E002',
+                )
+            )
+        if not settings.MKTG_URLS.get('ROOT'):
+            errors.append(
+                Error(
+                    'There is no ROOT defined in MKTG_URLS.',
+                    id='common.djangoapps.common_initialization.E003',
+                )
+            )
+    return errors

openedx/core/djangoapps/theming/apps.py

@@ -1,83 +1,11 @@
 
-import os
-import six
 from django.apps import AppConfig
-from django.conf import settings
-from django.core.checks import Error, Tags, register
 
 
 class ThemingConfig(AppConfig):
     name = 'openedx.core.djangoapps.theming'
     verbose_name = "Theming"
 
-
+    def ready(self):
-@register(Tags.compatibility)
+        # settings validations related to theming.
-def check_comprehensive_theme_settings(app_configs, **kwargs):
+        from . import checks
-    """
-    Checks the comprehensive theming theme directory settings.
-
-    Raises compatibility Errors upon:
-        - COMPREHENSIVE_THEME_DIRS is not a list
-        - theme dir path is not a string
-        - theme dir path is not an absolute path
-        - path specified in COMPREHENSIVE_THEME_DIRS does not exist
-
-    Returns:
-        List of any Errors.
-    """
-    if not getattr(settings, "ENABLE_COMPREHENSIVE_THEMING"):
-        # Only perform checks when comprehensive theming is enabled.
-        return []
-
-    errors = []
-
-    # COMPREHENSIVE_THEME_DIR is no longer supported - support has been removed.
-    if hasattr(settings, "COMPREHENSIVE_THEME_DIR"):
-        theme_dir = settings.COMPREHENSIVE_THEME_DIR
-
-        errors.append(
-            Error(
-                "COMPREHENSIVE_THEME_DIR setting has been removed in favor of COMPREHENSIVE_THEME_DIRS.",
-                hint='Transfer the COMPREHENSIVE_THEME_DIR value to COMPREHENSIVE_THEME_DIRS.',
-                obj=theme_dir,
-                id='openedx.core.djangoapps.theming.E001',
-            )
-        )
-
-    if hasattr(settings, "COMPREHENSIVE_THEME_DIRS"):
-        theme_dirs = settings.COMPREHENSIVE_THEME_DIRS
-
-        if not isinstance(theme_dirs, list):
-            errors.append(
-                Error(
-                    "COMPREHENSIVE_THEME_DIRS must be a list.",
-                    obj=theme_dirs,
-                    id='openedx.core.djangoapps.theming.E004',
-                )
-            )
-        if not all([isinstance(theme_dir, six.string_types) for theme_dir in theme_dirs]):
-            errors.append(
-                Error(
-                    "COMPREHENSIVE_THEME_DIRS must contain only strings.",
-                    obj=theme_dirs,
-                    id='openedx.core.djangoapps.theming.E005',
-                )
-            )
-        if not all([theme_dir.startswith("/") for theme_dir in theme_dirs]):
-            errors.append(
-                Error(
-                    "COMPREHENSIVE_THEME_DIRS must contain only absolute paths to themes dirs.",
-                    obj=theme_dirs,
-                    id='openedx.core.djangoapps.theming.E006',
-                )
-            )
-        if not all([os.path.isdir(theme_dir) for theme_dir in theme_dirs]):
-            errors.append(
-                Error(
-                    "COMPREHENSIVE_THEME_DIRS must contain valid paths.",
-                    obj=theme_dirs,
-                    id='openedx.core.djangoapps.theming.E007',
-                )
-            )
-
-    return errors

openedx/core/djangoapps/theming/checks.py

@@ -0,0 +1,79 @@
+"""
+Settings validations for the theming app
+"""
+import os
+import six
+from django.conf import settings
+from django.core.checks import Error, Tags, register
+
+
+@register(Tags.compatibility)
+def check_comprehensive_theme_settings(app_configs, **kwargs):
+    """
+    Checks the comprehensive theming theme directory settings.
+
+    Raises compatibility Errors upon:
+        - COMPREHENSIVE_THEME_DIRS is not a list
+        - theme dir path is not a string
+        - theme dir path is not an absolute path
+        - path specified in COMPREHENSIVE_THEME_DIRS does not exist
+
+    Returns:
+        List of any Errors.
+    """
+    if not getattr(settings, "ENABLE_COMPREHENSIVE_THEMING"):
+        # Only perform checks when comprehensive theming is enabled.
+        return []
+
+    errors = []
+
+    # COMPREHENSIVE_THEME_DIR is no longer supported - support has been removed.
+    if hasattr(settings, "COMPREHENSIVE_THEME_DIR"):
+        theme_dir = settings.COMPREHENSIVE_THEME_DIR
+
+        errors.append(
+            Error(
+                "COMPREHENSIVE_THEME_DIR setting has been removed in favor of COMPREHENSIVE_THEME_DIRS.",
+                hint='Transfer the COMPREHENSIVE_THEME_DIR value to COMPREHENSIVE_THEME_DIRS.',
+                obj=theme_dir,
+                id='openedx.core.djangoapps.theming.E001',
+            )
+        )
+
+    if hasattr(settings, "COMPREHENSIVE_THEME_DIRS"):
+        theme_dirs = settings.COMPREHENSIVE_THEME_DIRS
+
+        if not isinstance(theme_dirs, list):
+            errors.append(
+                Error(
+                    "COMPREHENSIVE_THEME_DIRS must be a list.",
+                    obj=theme_dirs,
+                    id='openedx.core.djangoapps.theming.E004',
+                )
+            )
+        if not all([isinstance(theme_dir, six.string_types) for theme_dir in theme_dirs]):
+            errors.append(
+                Error(
+                    "COMPREHENSIVE_THEME_DIRS must contain only strings.",
+                    obj=theme_dirs,
+                    id='openedx.core.djangoapps.theming.E005',
+                )
+            )
+        if not all([theme_dir.startswith("/") for theme_dir in theme_dirs]):
+            errors.append(
+                Error(
+                    "COMPREHENSIVE_THEME_DIRS must contain only absolute paths to themes dirs.",
+                    obj=theme_dirs,
+                    id='openedx.core.djangoapps.theming.E006',
+                )
+            )
+        if not all([os.path.isdir(theme_dir) for theme_dir in theme_dirs]):
+            errors.append(
+                Error(
+                    "COMPREHENSIVE_THEME_DIRS must contain valid paths.",
+                    obj=theme_dirs,
+                    id='openedx.core.djangoapps.theming.E007',
+                )
+            )
+
+    return errors