From 954f71f01d0b5527246fab40f7c9c59012c15a6b Mon Sep 17 00:00:00 2001 From: dansubak Date: Fri, 3 Oct 2025 13:46:52 -0400 Subject: [PATCH 01/10] WIP generated settings validation --- main/env_validator.py | 229 +++++++++++++++++++++++ main/management/commands/validate_env.py | 1 + main/settings.py | 12 ++ 3 files changed, 242 insertions(+) create mode 100644 main/env_validator.py create mode 100644 main/management/commands/validate_env.py diff --git a/main/env_validator.py b/main/env_validator.py new file mode 100644 index 0000000000..d3bbcaefa7 --- /dev/null +++ b/main/env_validator.py @@ -0,0 +1,229 @@ +""" +Environment variable validation utilities for detecting configuration discrepancies. +""" + +import logging +import os +import re +from pathlib import Path +from typing import Dict, List, Tuple, Optional + +logger = logging.getLogger(__name__) + +def strip_comment(val): + return val.split('#', 1)[0].strip() + +class EnvValidator: + """Validates environment variable configurations and reports discrepancies.""" + + def __init__(self, project_root: Optional[str] = None): + """ + Initialize the environment validator. + + Args: + project_root: Root directory of the project. Defaults to parent of current file. + """ + if project_root is None: + project_root = Path(__file__).parent.parent + else: + project_root = Path(project_root) + + self.project_root = project_root + self.env_dir = project_root / "env" + + def _parse_env_file(self, file_path: Path) -> Dict[str, dict]: + """ + Parse an environment file and return a dictionary of key-value pairs and any noqa-style directives. + + Args: + file_path: Path to the environment file + + Returns: + Dictionary mapping variable names to dicts with 'value' and optional 'directive'. + """ + env_vars = {} + + if not file_path.exists(): + return env_vars + + try: + with open(file_path, 'r', encoding='utf-8') as f: + for line_num, line in enumerate(f, 1): + line = line.strip() + + # Skip empty lines and comments + if not line or line.startswith('#'): + continue + + # Check for noqa-style directive at end of line + directive = None + if line.endswith("# local-required"): + directive = "local-required" + line = line[: -len("# local-required")].rstrip() + elif line.endswith("# suppress-warning"): + directive = "suppress-warning" + line = line[: -len("# suppress-warning")].rstrip() + + # Parse regular env variables + if '=' in line: + key, value = line.split('=', 1) + key = key.strip() + value = value.strip() + + env_vars[key] = {'value': value} + if directive: + env_vars[key]['directive'] = directive + + except Exception as e: + logger.warning(f"Error parsing env file {file_path}: {e}") + + return env_vars + + def _get_env_file_pairs(self) -> List[Tuple[str, Path, Path, Path]]: + """ + Get pairs of environment files to compare. + + Returns: + List of tuples: (env_type, base_env_path, local_env_path, example_env_path) + """ + pairs = [] + + # Define the environment file patterns + env_patterns = [ + ("backend", "backend.env", "backend.local.env", "backend.local.example.env"), + ("frontend", "frontend.env", "frontend.local.env", "frontend.local.example.env"), + ("shared", "shared.env", "shared.local.env", "shared.local.example.env"), + ] + + for env_type, base_name, local_name, example_name in env_patterns: + base_path = self.env_dir / base_name + local_path = self.env_dir / local_name + example_path = self.env_dir / example_name + + # Include if any of the files exist (we'll check existence in validation methods) + if base_path.exists() or local_path.exists() or example_path.exists(): + pairs.append((env_type, base_path, local_path, example_path)) + + return pairs + + def check_example_overrides(self) -> List[str]: + """ + Check for settings present in example files but not in base env files. + This identifies non-standard settings that are overridden in the environment. + + Returns: + List of warning messages + """ + warnings = [] + + for env_type, base_path, local_path, example_path in self._get_env_file_pairs(): + if not example_path.exists(): + continue + + base_vars = self._parse_env_file(base_path) if base_path.exists() else {} + example_vars = self._parse_env_file(example_path) + + example_only = set(example_vars.keys()) - set(base_vars.keys()) + + for var_name in example_only: + # Only warn if the variable is present in local file + if local_path.exists(): + local_vars = self._parse_env_file(local_path) + if var_name in local_vars: + example_value = example_vars[var_name]['value'] + local_value = local_vars[var_name]['value'] + if strip_comment(local_value) == strip_comment(example_value): + continue + warnings.append( + f"⚠️ {env_type.upper()}: Variable '{var_name}' is set in {local_path.name} " + f"but not defined in {base_path.name}. This overrides a non-standard setting " + f"from {example_path.name}. Local value: '{local_value}', " + f"Example value: '{example_value}'" + ) + return warnings + + def check_local_overrides(self) -> List[str]: + """ + Check for settings in local files that differ from or are absent in base files. + + Returns: + List of warning messages + """ + warnings = [] + + for env_type, base_path, local_path, example_path in self._get_env_file_pairs(): + if not local_path.exists(): + continue + + base_vars = self._parse_env_file(base_path) if base_path.exists() else {} + local_vars = self._parse_env_file(local_path) + + for var_name, local_info in local_vars.items(): + local_value = local_info['value'] + suppress = local_info.get('directive') == 'suppress-warning' + if var_name not in base_vars: + if suppress: + continue + warnings.append( + f"⚠️ {env_type.upper()}: Variable '{var_name}' is set in {local_path.name} " + f"(value: '{local_value}') but not defined in {base_path.name}. " + f"Consider adding a default value to {base_path.name} if this should be a standard setting." + ) + else: + base_value = base_vars[var_name]['value'] + # Compare values ignoring anything after a "#" symbol + if strip_comment(base_value) == strip_comment(local_value): + continue # No warning if values match (ignoring comments) + # Suppress warning if base file has # local-required + if base_vars[var_name].get('directive') == 'local-required': + continue + warnings.append( + f"⚠️ {env_type.upper()}: Variable '{var_name}' is overridden locally. " + f"Base value: '{base_value}', Local value: '{local_value}'. " + f"Ensure the default in {base_path.name} is appropriate." + ) + return warnings + + def validate_all(self) -> List[str]: + """ + Run all environment validation checks. + + Returns: + List of all warning messages + """ + warnings = [] + warnings.extend(self.check_example_overrides()) + warnings.extend(self.check_local_overrides()) + return warnings + + def log_warnings(self, warnings: List[str]) -> None: + """ + Log warning messages. + + Args: + warnings: List of warning messages to log + """ + if not warnings: + logger.info("✅ Environment validation passed - no configuration discrepancies found.") + return + + logger.warning("Environment Configuration Warnings:") + logger.warning("=" * 60) + for warning in warnings: + logger.warning(warning) + logger.warning("=" * 60) + logger.warning( + f"Found {len(warnings)} environment configuration issue(s). " + "Review your environment files to ensure proper configuration." + ) + + +def validate_environment_on_startup(): + """ + Main function to validate environment configuration on startup. + This can be called from Django settings or management commands. + """ + validator = EnvValidator() + warnings = validator.validate_all() + validator.log_warnings(warnings) + return warnings diff --git a/main/management/commands/validate_env.py b/main/management/commands/validate_env.py new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/main/management/commands/validate_env.py @@ -0,0 +1 @@ + diff --git a/main/settings.py b/main/settings.py index 8d37ed4fc9..7518a4bbec 100644 --- a/main/settings.py +++ b/main/settings.py @@ -55,6 +55,17 @@ profiles_sample_rate=SENTRY_PROFILES_SAMPLE_RATE, ) +# Validate environment configuration on startup +# Skip validation during testing or when explicitly disabled +if not get_bool("SKIP_ENV_VALIDATION", False) and "test" not in os.environ.get("DJANGO_SETTINGS_MODULE", ""): + try: + from main.env_validator import validate_environment_on_startup + validate_environment_on_startup() + except ImportError as e: + log.warning(f"Could not import environment validator: {e}") + except Exception as e: + log.warning(f"Environment validation failed: {e}") + BASE_DIR = os.path.dirname( # noqa: PTH120 os.path.dirname(os.path.abspath(__file__)) # noqa: PTH100, PTH120 ) @@ -866,3 +877,4 @@ def get_all_config_keys(): OPENTELEMETRY_TRACES_BATCH_SIZE = get_int("OPENTELEMETRY_TRACES_BATCH_SIZE", 512) OPENTELEMETRY_EXPORT_TIMEOUT_MS = get_int("OPENTELEMETRY_EXPORT_TIMEOUT_MS", 5000) CANVAS_TUTORBOT_FOLDER = get_string("CANVAS_TUTORBOT_FOLDER", "web_resources/ai/tutor/") + From 3c0c7ceeaf3404cdfa906f16a3459a3e62c36ab1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 3 Oct 2025 20:12:58 +0000 Subject: [PATCH 02/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- main/env_validator.py | 62 ++++++++++++++---------- main/management/commands/validate_env.py | 1 - main/settings.py | 6 ++- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/main/env_validator.py b/main/env_validator.py index d3bbcaefa7..096ed7aee3 100644 --- a/main/env_validator.py +++ b/main/env_validator.py @@ -3,15 +3,15 @@ """ import logging -import os -import re from pathlib import Path -from typing import Dict, List, Tuple, Optional +from typing import Optional logger = logging.getLogger(__name__) + def strip_comment(val): - return val.split('#', 1)[0].strip() + return val.split("#", 1)[0].strip() + class EnvValidator: """Validates environment variable configurations and reports discrepancies.""" @@ -31,7 +31,7 @@ def __init__(self, project_root: Optional[str] = None): self.project_root = project_root self.env_dir = project_root / "env" - def _parse_env_file(self, file_path: Path) -> Dict[str, dict]: + def _parse_env_file(self, file_path: Path) -> dict[str, dict]: """ Parse an environment file and return a dictionary of key-value pairs and any noqa-style directives. @@ -47,12 +47,12 @@ def _parse_env_file(self, file_path: Path) -> Dict[str, dict]: return env_vars try: - with open(file_path, 'r', encoding='utf-8') as f: + with open(file_path, encoding="utf-8") as f: for line_num, line in enumerate(f, 1): line = line.strip() # Skip empty lines and comments - if not line or line.startswith('#'): + if not line or line.startswith("#"): continue # Check for noqa-style directive at end of line @@ -65,21 +65,21 @@ def _parse_env_file(self, file_path: Path) -> Dict[str, dict]: line = line[: -len("# suppress-warning")].rstrip() # Parse regular env variables - if '=' in line: - key, value = line.split('=', 1) + if "=" in line: + key, value = line.split("=", 1) key = key.strip() value = value.strip() - env_vars[key] = {'value': value} + env_vars[key] = {"value": value} if directive: - env_vars[key]['directive'] = directive + env_vars[key]["directive"] = directive except Exception as e: logger.warning(f"Error parsing env file {file_path}: {e}") return env_vars - def _get_env_file_pairs(self) -> List[Tuple[str, Path, Path, Path]]: + def _get_env_file_pairs(self) -> list[tuple[str, Path, Path, Path]]: """ Get pairs of environment files to compare. @@ -90,8 +90,18 @@ def _get_env_file_pairs(self) -> List[Tuple[str, Path, Path, Path]]: # Define the environment file patterns env_patterns = [ - ("backend", "backend.env", "backend.local.env", "backend.local.example.env"), - ("frontend", "frontend.env", "frontend.local.env", "frontend.local.example.env"), + ( + "backend", + "backend.env", + "backend.local.env", + "backend.local.example.env", + ), + ( + "frontend", + "frontend.env", + "frontend.local.env", + "frontend.local.example.env", + ), ("shared", "shared.env", "shared.local.env", "shared.local.example.env"), ] @@ -106,7 +116,7 @@ def _get_env_file_pairs(self) -> List[Tuple[str, Path, Path, Path]]: return pairs - def check_example_overrides(self) -> List[str]: + def check_example_overrides(self) -> list[str]: """ Check for settings present in example files but not in base env files. This identifies non-standard settings that are overridden in the environment. @@ -130,8 +140,8 @@ def check_example_overrides(self) -> List[str]: if local_path.exists(): local_vars = self._parse_env_file(local_path) if var_name in local_vars: - example_value = example_vars[var_name]['value'] - local_value = local_vars[var_name]['value'] + example_value = example_vars[var_name]["value"] + local_value = local_vars[var_name]["value"] if strip_comment(local_value) == strip_comment(example_value): continue warnings.append( @@ -142,7 +152,7 @@ def check_example_overrides(self) -> List[str]: ) return warnings - def check_local_overrides(self) -> List[str]: + def check_local_overrides(self) -> list[str]: """ Check for settings in local files that differ from or are absent in base files. @@ -159,8 +169,8 @@ def check_local_overrides(self) -> List[str]: local_vars = self._parse_env_file(local_path) for var_name, local_info in local_vars.items(): - local_value = local_info['value'] - suppress = local_info.get('directive') == 'suppress-warning' + local_value = local_info["value"] + suppress = local_info.get("directive") == "suppress-warning" if var_name not in base_vars: if suppress: continue @@ -170,12 +180,12 @@ def check_local_overrides(self) -> List[str]: f"Consider adding a default value to {base_path.name} if this should be a standard setting." ) else: - base_value = base_vars[var_name]['value'] + base_value = base_vars[var_name]["value"] # Compare values ignoring anything after a "#" symbol if strip_comment(base_value) == strip_comment(local_value): continue # No warning if values match (ignoring comments) # Suppress warning if base file has # local-required - if base_vars[var_name].get('directive') == 'local-required': + if base_vars[var_name].get("directive") == "local-required": continue warnings.append( f"⚠️ {env_type.upper()}: Variable '{var_name}' is overridden locally. " @@ -184,7 +194,7 @@ def check_local_overrides(self) -> List[str]: ) return warnings - def validate_all(self) -> List[str]: + def validate_all(self) -> list[str]: """ Run all environment validation checks. @@ -196,7 +206,7 @@ def validate_all(self) -> List[str]: warnings.extend(self.check_local_overrides()) return warnings - def log_warnings(self, warnings: List[str]) -> None: + def log_warnings(self, warnings: list[str]) -> None: """ Log warning messages. @@ -204,7 +214,9 @@ def log_warnings(self, warnings: List[str]) -> None: warnings: List of warning messages to log """ if not warnings: - logger.info("✅ Environment validation passed - no configuration discrepancies found.") + logger.info( + "✅ Environment validation passed - no configuration discrepancies found." + ) return logger.warning("Environment Configuration Warnings:") diff --git a/main/management/commands/validate_env.py b/main/management/commands/validate_env.py index 8b13789179..e69de29bb2 100644 --- a/main/management/commands/validate_env.py +++ b/main/management/commands/validate_env.py @@ -1 +0,0 @@ - diff --git a/main/settings.py b/main/settings.py index 7518a4bbec..50ef438b21 100644 --- a/main/settings.py +++ b/main/settings.py @@ -57,9 +57,12 @@ # Validate environment configuration on startup # Skip validation during testing or when explicitly disabled -if not get_bool("SKIP_ENV_VALIDATION", False) and "test" not in os.environ.get("DJANGO_SETTINGS_MODULE", ""): +if not get_bool("SKIP_ENV_VALIDATION", False) and "test" not in os.environ.get( + "DJANGO_SETTINGS_MODULE", "" +): try: from main.env_validator import validate_environment_on_startup + validate_environment_on_startup() except ImportError as e: log.warning(f"Could not import environment validator: {e}") @@ -877,4 +880,3 @@ def get_all_config_keys(): OPENTELEMETRY_TRACES_BATCH_SIZE = get_int("OPENTELEMETRY_TRACES_BATCH_SIZE", 512) OPENTELEMETRY_EXPORT_TIMEOUT_MS = get_int("OPENTELEMETRY_EXPORT_TIMEOUT_MS", 5000) CANVAS_TUTORBOT_FOLDER = get_string("CANVAS_TUTORBOT_FOLDER", "web_resources/ai/tutor/") - From 199ac6bcde04bb823731735114bee36b6dcfbfbd Mon Sep 17 00:00:00 2001 From: dansubak Date: Wed, 8 Oct 2025 12:19:25 -0400 Subject: [PATCH 03/10] Precommit fixes --- main/env_validator.py | 102 +++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/main/env_validator.py b/main/env_validator.py index 096ed7aee3..974592ae25 100644 --- a/main/env_validator.py +++ b/main/env_validator.py @@ -21,7 +21,8 @@ def __init__(self, project_root: Optional[str] = None): Initialize the environment validator. Args: - project_root: Root directory of the project. Defaults to parent of current file. + project_root: Root directory of the project. + If None, defaults to parent of this file's directory. """ if project_root is None: project_root = Path(__file__).parent.parent @@ -33,49 +34,47 @@ def __init__(self, project_root: Optional[str] = None): def _parse_env_file(self, file_path: Path) -> dict[str, dict]: """ - Parse an environment file and return a dictionary of key-value pairs and any noqa-style directives. + Parse an environment file and return a dictionary of key-value pairs + and any noqa-style directives. Args: file_path: Path to the environment file Returns: - Dictionary mapping variable names to dicts with 'value' and optional 'directive'. + Dictionary mapping variable names to dicts with 'value' + and optional 'directive'. """ env_vars = {} if not file_path.exists(): return env_vars - try: - with open(file_path, encoding="utf-8") as f: - for line_num, line in enumerate(f, 1): - line = line.strip() - - # Skip empty lines and comments - if not line or line.startswith("#"): - continue - - # Check for noqa-style directive at end of line - directive = None - if line.endswith("# local-required"): - directive = "local-required" - line = line[: -len("# local-required")].rstrip() - elif line.endswith("# suppress-warning"): - directive = "suppress-warning" - line = line[: -len("# suppress-warning")].rstrip() - - # Parse regular env variables - if "=" in line: - key, value = line.split("=", 1) - key = key.strip() - value = value.strip() - - env_vars[key] = {"value": value} - if directive: - env_vars[key]["directive"] = directive - - except Exception as e: - logger.warning(f"Error parsing env file {file_path}: {e}") + with Path.open(file_path, encoding="utf-8") as f: + for original_line in f: + line = original_line.strip() + + # Skip empty lines and comments + if not line or line.startswith("#"): + continue + + # Check for noqa-style directive at end of line + directive = None + if line.endswith("# local-required"): + directive = "local-required" + line = line[: -len("# local-required")].rstrip() + elif line.endswith("# suppress-warning"): + directive = "suppress-warning" + line = line[: -len("# suppress-warning")].rstrip() + + # Parse regular env variables + if "=" in line: + key, value = line.split("=", 1) + key = key.strip() + value = value.strip() + + env_vars[key] = {"value": value} + if directive: + env_vars[key]["directive"] = directive return env_vars @@ -110,7 +109,6 @@ def _get_env_file_pairs(self) -> list[tuple[str, Path, Path, Path]]: local_path = self.env_dir / local_name example_path = self.env_dir / example_name - # Include if any of the files exist (we'll check existence in validation methods) if base_path.exists() or local_path.exists() or example_path.exists(): pairs.append((env_type, base_path, local_path, example_path)) @@ -145,9 +143,12 @@ def check_example_overrides(self) -> list[str]: if strip_comment(local_value) == strip_comment(example_value): continue warnings.append( - f"⚠️ {env_type.upper()}: Variable '{var_name}' is set in {local_path.name} " - f"but not defined in {base_path.name}. This overrides a non-standard setting " - f"from {example_path.name}. Local value: '{local_value}', " + f"⚠️ {env_type.upper()}: Variable " + f"'{var_name}' is set in {local_path.name} " + f"but not defined in {base_path.name}. " + f"This overrides a non-standard setting " + f"from {example_path.name}. " + f"Local value: '{local_value}', " f"Example value: '{example_value}'" ) return warnings @@ -161,7 +162,7 @@ def check_local_overrides(self) -> list[str]: """ warnings = [] - for env_type, base_path, local_path, example_path in self._get_env_file_pairs(): + for env_type, base_path, local_path, _ in self._get_env_file_pairs(): if not local_path.exists(): continue @@ -175,9 +176,12 @@ def check_local_overrides(self) -> list[str]: if suppress: continue warnings.append( - f"⚠️ {env_type.upper()}: Variable '{var_name}' is set in {local_path.name} " - f"(value: '{local_value}') but not defined in {base_path.name}. " - f"Consider adding a default value to {base_path.name} if this should be a standard setting." + f"⚠️ {env_type.upper()}: Variable " + f"'{var_name}' is set in {local_path.name} " + f"(value: '{local_value}') but " + f"not defined in {base_path.name}. " + f"Consider adding a default value to " + f"{base_path.name} if this should be a standard setting." ) else: base_value = base_vars[var_name]["value"] @@ -188,8 +192,10 @@ def check_local_overrides(self) -> list[str]: if base_vars[var_name].get("directive") == "local-required": continue warnings.append( - f"⚠️ {env_type.upper()}: Variable '{var_name}' is overridden locally. " - f"Base value: '{base_value}', Local value: '{local_value}'. " + f"⚠️ {env_type.upper()}: Variable " + f"'{var_name}' is overridden locally. " + f"Base value: '{base_value}', " + f"Local value: '{local_value}'. " f"Ensure the default in {base_path.name} is appropriate." ) return warnings @@ -215,7 +221,8 @@ def log_warnings(self, warnings: list[str]) -> None: """ if not warnings: logger.info( - "✅ Environment validation passed - no configuration discrepancies found." + "✅ Environment validation passed - " + "no configuration discrepancies found." ) return @@ -225,14 +232,15 @@ def log_warnings(self, warnings: list[str]) -> None: logger.warning(warning) logger.warning("=" * 60) logger.warning( - f"Found {len(warnings)} environment configuration issue(s). " - "Review your environment files to ensure proper configuration." + "Found %s environment configuration issue(s). " + "Review your environment files to ensure proper configuration.", + len(warnings), ) def validate_environment_on_startup(): """ - Main function to validate environment configuration on startup. + Validate environment configuration on startup. This can be called from Django settings or management commands. """ validator = EnvValidator() From 2f8f854e54b843a9a743557d5f4391b6d96da5db Mon Sep 17 00:00:00 2001 From: dansubak Date: Wed, 8 Oct 2025 12:27:12 -0400 Subject: [PATCH 04/10] Precommit fixes --- main/settings.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/main/settings.py b/main/settings.py index 50ef438b21..8beaf421a5 100644 --- a/main/settings.py +++ b/main/settings.py @@ -57,17 +57,16 @@ # Validate environment configuration on startup # Skip validation during testing or when explicitly disabled -if not get_bool("SKIP_ENV_VALIDATION", False) and "test" not in os.environ.get( +if not get_bool("SKIP_ENV_VALIDATION", default=False) and "test" not in os.environ.get( "DJANGO_SETTINGS_MODULE", "" ): try: from main.env_validator import validate_environment_on_startup validate_environment_on_startup() - except ImportError as e: - log.warning(f"Could not import environment validator: {e}") - except Exception as e: - log.warning(f"Environment validation failed: {e}") + except Exception as e: # noqa: BLE001 + # We don't want to block if validation fails. + log.warning("Environment validation failed: %s", e) BASE_DIR = os.path.dirname( # noqa: PTH120 os.path.dirname(os.path.abspath(__file__)) # noqa: PTH100, PTH120 From b9f7a40eb9fc558e2f8bea66c8f49b59ead3111d Mon Sep 17 00:00:00 2001 From: dansubak Date: Wed, 8 Oct 2025 12:45:01 -0400 Subject: [PATCH 05/10] Remove settings module check --- main/settings.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/main/settings.py b/main/settings.py index 8beaf421a5..13efcaee43 100644 --- a/main/settings.py +++ b/main/settings.py @@ -57,9 +57,7 @@ # Validate environment configuration on startup # Skip validation during testing or when explicitly disabled -if not get_bool("SKIP_ENV_VALIDATION", default=False) and "test" not in os.environ.get( - "DJANGO_SETTINGS_MODULE", "" -): +if not get_bool("SKIP_ENV_VALIDATION", default=False): try: from main.env_validator import validate_environment_on_startup From 18f9b538a76ffaafb433f5ce46482a16b03465dc Mon Sep 17 00:00:00 2001 From: dansubak Date: Mon, 27 Oct 2025 14:59:02 -0400 Subject: [PATCH 06/10] Add behaviors and rules for settings use --- main/env_validator.py | 26 ++++++++++++++++++++++++++ main/settings.py | 22 +++++++++++----------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/main/env_validator.py b/main/env_validator.py index 974592ae25..d21b326408 100644 --- a/main/env_validator.py +++ b/main/env_validator.py @@ -13,6 +13,32 @@ def strip_comment(val): return val.split("#", 1)[0].strip() +""" +This attempts to enforce the following rules around env files: +- Base env files (i.e. backend.env) will contain default settings +that are safe for all development environments +- Local env files (i.e. backend.local.env) may contain overrides and +settings which cannot have a sensible default (i.e. developer-specific API keys). +It should contain everything in the example file as well +as anything intentionally overridden +- Example env files list only settings which cannot have a sensible default. +It should contain the minimum required set of settings values +necessary to get the application running. + + +Behavior we want to validate: +1) If a setting is in the example file or local file, but +not the base file it might be missing a default and we emit a warning. +1a) If a setting is in the example file or local file but not the +base because there's no sensible default (i.e. values are specific to a user or local) +we allow a "# local-required" comment on example file to suppress the warning +2) If a setting is in the local file and the base file, but the values +differ we emit a warning. This is to keep users from accidentally overriding defaults +3) If a setting is in the example file but not the local file, we emit a +warning. This indicates that we are likely missing a required local-specific setting +""" + + class EnvValidator: """Validates environment variable configurations and reports discrepancies.""" diff --git a/main/settings.py b/main/settings.py index 13efcaee43..ebb024b039 100644 --- a/main/settings.py +++ b/main/settings.py @@ -55,17 +55,6 @@ profiles_sample_rate=SENTRY_PROFILES_SAMPLE_RATE, ) -# Validate environment configuration on startup -# Skip validation during testing or when explicitly disabled -if not get_bool("SKIP_ENV_VALIDATION", default=False): - try: - from main.env_validator import validate_environment_on_startup - - validate_environment_on_startup() - except Exception as e: # noqa: BLE001 - # We don't want to block if validation fails. - log.warning("Environment validation failed: %s", e) - BASE_DIR = os.path.dirname( # noqa: PTH120 os.path.dirname(os.path.abspath(__file__)) # noqa: PTH100, PTH120 ) @@ -877,3 +866,14 @@ def get_all_config_keys(): OPENTELEMETRY_TRACES_BATCH_SIZE = get_int("OPENTELEMETRY_TRACES_BATCH_SIZE", 512) OPENTELEMETRY_EXPORT_TIMEOUT_MS = get_int("OPENTELEMETRY_EXPORT_TIMEOUT_MS", 5000) CANVAS_TUTORBOT_FOLDER = get_string("CANVAS_TUTORBOT_FOLDER", "web_resources/ai/tutor/") + +# Validate environment configuration on startup +# Skip validation during testing or when explicitly disabled +if not get_bool("SKIP_ENV_VALIDATION", default=False): + try: + from main.env_validator import validate_environment_on_startup + + validate_environment_on_startup() + except Exception as e: # noqa: BLE001 + # We don't want to block if validation fails. + log.warning("Environment validation failed: %s", e) From ffc0f72274edf9eaa70305ebcf7b6535a4474242 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 27 Oct 2025 19:01:58 +0000 Subject: [PATCH 07/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- main/env_validator.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main/env_validator.py b/main/env_validator.py index d21b326408..88eb077219 100644 --- a/main/env_validator.py +++ b/main/env_validator.py @@ -4,7 +4,6 @@ import logging from pathlib import Path -from typing import Optional logger = logging.getLogger(__name__) @@ -42,7 +41,7 @@ def strip_comment(val): class EnvValidator: """Validates environment variable configurations and reports discrepancies.""" - def __init__(self, project_root: Optional[str] = None): + def __init__(self, project_root: str | None = None): """ Initialize the environment validator. From e01791d60f4e51d879208526f8d8c7fc4ff75472 Mon Sep 17 00:00:00 2001 From: dansubak Date: Wed, 29 Oct 2025 10:41:37 -0400 Subject: [PATCH 08/10] scaffolded out tests --- main/test_env_validator.py | 86 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 main/test_env_validator.py diff --git a/main/test_env_validator.py b/main/test_env_validator.py new file mode 100644 index 0000000000..d8d7aefa21 --- /dev/null +++ b/main/test_env_validator.py @@ -0,0 +1,86 @@ +import tempfile +from pathlib import Path + +from main.env_validator import EnvValidator + + +class TestEnvValidator: + def setup_env(self, files, tmpdir): + file_paths = {} + for fname, lines in files.items(): + path = Path(tmpdir) / fname + with Path.open(path, "w", encoding="utf-8") as f: + f.write("\n".join(lines)) + file_paths[fname] = path + return file_paths + + def test_warn_if_setting_in_example_not_base(self): + files = { + "backend.env": ["BASE=1"], + "backend.local.env": ["BASE=1", "EXTRA=foo"], + "backend.local.example.env": ["EXTRA=bar"], + } + with tempfile.TemporaryDirectory() as tmpdir: + file_paths = self.setup_env(files, tmpdir) + validator = EnvValidator( + base_env_path=file_paths["backend.env"], + local_env_path=file_paths["backend.local.env"], + example_env_path=file_paths["backend.local.example.env"], + ) + warnings = validator.validate_all() + assert any( + "EXTRA" in w and "not defined in backend.env" in w for w in warnings + ) + + def test_suppress_warning_with_local_required(self): + files = { + "backend.env": ["BASE=1"], + "backend.local.env": ["BASE=1", "EXTRA=foo"], + "backend.local.example.env": ["EXTRA=bar # local-required"], + } + with tempfile.TemporaryDirectory() as tmpdir: + file_paths = self.setup_env(files, tmpdir) + validator = EnvValidator( + base_env_path=file_paths["backend.env"], + local_env_path=file_paths["backend.local.env"], + example_env_path=file_paths["backend.local.example.env"], + ) + warnings = validator.validate_all() + assert not any( + "EXTRA" in w and "not defined in backend.env" in w for w in warnings + ) + + def test_warn_if_local_overrides_base(self): + files = { + "backend.env": ["BASE=1"], + "backend.local.env": ["BASE=2"], + "backend.local.example.env": ["BASE=1"], + } + with tempfile.TemporaryDirectory() as tmpdir: + file_paths = self.setup_env(files, tmpdir) + validator = EnvValidator( + base_env_path=file_paths["backend.env"], + local_env_path=file_paths["backend.local.env"], + example_env_path=file_paths["backend.local.example.env"], + ) + warnings = validator.validate_all() + assert any("BASE" in w and "overridden locally" in w for w in warnings) + + def test_warn_if_example_not_in_local(self): + files = { + "backend.env": ["BASE=1"], + "backend.local.env": ["BASE=1"], + "backend.local.example.env": ["BASE=1", "EXTRA=bar"], + } + with tempfile.TemporaryDirectory() as tmpdir: + file_paths = self.setup_env(files, tmpdir) + validator = EnvValidator( + base_env_path=file_paths["backend.env"], + local_env_path=file_paths["backend.local.env"], + example_env_path=file_paths["backend.local.example.env"], + ) + warnings = validator.validate_all() + assert any( + "EXTRA" in w and "missing a required local-specific setting" in w + for w in warnings + ) From e6c6b6b48fe0ef42a6c1df4d989c37f6542f8011 Mon Sep 17 00:00:00 2001 From: dansubak Date: Wed, 29 Oct 2025 11:09:22 -0400 Subject: [PATCH 09/10] Test --- main/env_validator.py | 9 ++++++--- main/test_env_validator.py | 15 ++++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/main/env_validator.py b/main/env_validator.py index 88eb077219..5bb8bdc4f5 100644 --- a/main/env_validator.py +++ b/main/env_validator.py @@ -37,6 +37,9 @@ def strip_comment(val): warning. This indicates that we are likely missing a required local-specific setting """ +# NB: This runs way too frequently (i.e. in tests). Need to figure out how to +# Run only on interactive startup + class EnvValidator: """Validates environment variable configurations and reports discrepancies.""" @@ -103,7 +106,7 @@ def _parse_env_file(self, file_path: Path) -> dict[str, dict]: return env_vars - def _get_env_file_pairs(self) -> list[tuple[str, Path, Path, Path]]: + def get_env_file_pairs(self) -> list[tuple[str, Path, Path, Path]]: """ Get pairs of environment files to compare. @@ -149,7 +152,7 @@ def check_example_overrides(self) -> list[str]: """ warnings = [] - for env_type, base_path, local_path, example_path in self._get_env_file_pairs(): + for env_type, base_path, local_path, example_path in self.get_env_file_pairs(): if not example_path.exists(): continue @@ -187,7 +190,7 @@ def check_local_overrides(self) -> list[str]: """ warnings = [] - for env_type, base_path, local_path, _ in self._get_env_file_pairs(): + for env_type, base_path, local_path, _ in self.get_env_file_pairs(): if not local_path.exists(): continue diff --git a/main/test_env_validator.py b/main/test_env_validator.py index d8d7aefa21..f6ce738e64 100644 --- a/main/test_env_validator.py +++ b/main/test_env_validator.py @@ -22,11 +22,16 @@ def test_warn_if_setting_in_example_not_base(self): } with tempfile.TemporaryDirectory() as tmpdir: file_paths = self.setup_env(files, tmpdir) - validator = EnvValidator( - base_env_path=file_paths["backend.env"], - local_env_path=file_paths["backend.local.env"], - example_env_path=file_paths["backend.local.example.env"], - ) + env_file_pairs = [ + ( + "backend", + file_paths["backend.env"], + file_paths["backend.local.env"], + file_paths["backend.local.example.env"], + ) + ] + validator = EnvValidator() + validator.get_env_file_pairs = lambda: env_file_pairs warnings = validator.validate_all() assert any( "EXTRA" in w and "not defined in backend.env" in w for w in warnings From 5b81cc3ad639436b3a4b1a50af721f986fc33319 Mon Sep 17 00:00:00 2001 From: dansubak Date: Wed, 29 Oct 2025 11:53:53 -0400 Subject: [PATCH 10/10] Test tweaks, notes about logical fixes needed --- main/env_validator.py | 43 ++++++++++++++++----------------- main/test_env_validator.py | 49 ++++++++++++++++++++++++-------------- 2 files changed, 52 insertions(+), 40 deletions(-) diff --git a/main/env_validator.py b/main/env_validator.py index 5bb8bdc4f5..798984b1a8 100644 --- a/main/env_validator.py +++ b/main/env_validator.py @@ -39,6 +39,8 @@ def strip_comment(val): # NB: This runs way too frequently (i.e. in tests). Need to figure out how to # Run only on interactive startup +# Also rewrite so this only allows directives in the expected files +# (i.e. local-required only in example files) class EnvValidator: @@ -153,9 +155,6 @@ def check_example_overrides(self) -> list[str]: warnings = [] for env_type, base_path, local_path, example_path in self.get_env_file_pairs(): - if not example_path.exists(): - continue - base_vars = self._parse_env_file(base_path) if base_path.exists() else {} example_vars = self._parse_env_file(example_path) @@ -163,22 +162,24 @@ def check_example_overrides(self) -> list[str]: for var_name in example_only: # Only warn if the variable is present in local file - if local_path.exists(): - local_vars = self._parse_env_file(local_path) - if var_name in local_vars: - example_value = example_vars[var_name]["value"] - local_value = local_vars[var_name]["value"] - if strip_comment(local_value) == strip_comment(example_value): - continue - warnings.append( - f"⚠️ {env_type.upper()}: Variable " - f"'{var_name}' is set in {local_path.name} " - f"but not defined in {base_path.name}. " - f"This overrides a non-standard setting " - f"from {example_path.name}. " - f"Local value: '{local_value}', " - f"Example value: '{example_value}'" - ) + local_vars = self._parse_env_file(local_path) + if var_name in local_vars: + example_value = example_vars[var_name]["value"] + local_value = local_vars[var_name]["value"] + if strip_comment(local_value) == strip_comment(example_value): + continue + # Suppress warning if example file cannot have a reasonable default + if example_vars[var_name].get("directive") == "local-required": + continue + warnings.append( + f"⚠️ {env_type.upper()}: Variable " + f"'{var_name}' is set in {local_path.name} " + f"but not defined in {base_path.name}. " + f"This overrides a non-standard setting " + f"from {example_path.name}. " + f"Local value: '{local_value}', " + f"Example value: '{example_value}'" + ) return warnings def check_local_overrides(self) -> list[str]: @@ -200,6 +201,7 @@ def check_local_overrides(self) -> list[str]: for var_name, local_info in local_vars.items(): local_value = local_info["value"] suppress = local_info.get("directive") == "suppress-warning" + if var_name not in base_vars: if suppress: continue @@ -216,9 +218,6 @@ def check_local_overrides(self) -> list[str]: # Compare values ignoring anything after a "#" symbol if strip_comment(base_value) == strip_comment(local_value): continue # No warning if values match (ignoring comments) - # Suppress warning if base file has # local-required - if base_vars[var_name].get("directive") == "local-required": - continue warnings.append( f"⚠️ {env_type.upper()}: Variable " f"'{var_name}' is overridden locally. " diff --git a/main/test_env_validator.py b/main/test_env_validator.py index f6ce738e64..5f30a246bc 100644 --- a/main/test_env_validator.py +++ b/main/test_env_validator.py @@ -45,15 +45,18 @@ def test_suppress_warning_with_local_required(self): } with tempfile.TemporaryDirectory() as tmpdir: file_paths = self.setup_env(files, tmpdir) - validator = EnvValidator( - base_env_path=file_paths["backend.env"], - local_env_path=file_paths["backend.local.env"], - example_env_path=file_paths["backend.local.example.env"], - ) + env_file_pairs = [ + ( + "backend", + file_paths["backend.env"], + file_paths["backend.local.env"], + file_paths["backend.local.example.env"], + ) + ] + validator = EnvValidator() + validator.get_env_file_pairs = lambda: env_file_pairs warnings = validator.validate_all() - assert not any( - "EXTRA" in w and "not defined in backend.env" in w for w in warnings - ) + assert len(warnings) == 0 def test_warn_if_local_overrides_base(self): files = { @@ -63,11 +66,16 @@ def test_warn_if_local_overrides_base(self): } with tempfile.TemporaryDirectory() as tmpdir: file_paths = self.setup_env(files, tmpdir) - validator = EnvValidator( - base_env_path=file_paths["backend.env"], - local_env_path=file_paths["backend.local.env"], - example_env_path=file_paths["backend.local.example.env"], - ) + env_file_pairs = [ + ( + "backend", + file_paths["backend.env"], + file_paths["backend.local.env"], + file_paths["backend.local.example.env"], + ) + ] + validator = EnvValidator() + validator.get_env_file_pairs = lambda: env_file_pairs warnings = validator.validate_all() assert any("BASE" in w and "overridden locally" in w for w in warnings) @@ -79,11 +87,16 @@ def test_warn_if_example_not_in_local(self): } with tempfile.TemporaryDirectory() as tmpdir: file_paths = self.setup_env(files, tmpdir) - validator = EnvValidator( - base_env_path=file_paths["backend.env"], - local_env_path=file_paths["backend.local.env"], - example_env_path=file_paths["backend.local.example.env"], - ) + env_file_pairs = [ + ( + "backend", + file_paths["backend.env"], + file_paths["backend.local.env"], + file_paths["backend.local.example.env"], + ) + ] + validator = EnvValidator() + validator.get_env_file_pairs = lambda: env_file_pairs warnings = validator.validate_all() assert any( "EXTRA" in w and "missing a required local-specific setting" in w