diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 2e01ba5..9a50283 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -10,24 +10,42 @@ This document provides a comprehensive reference for all configuration options i ### Configuration Sources -Aniworld uses a layered configuration system: +Aniworld uses a layered configuration system with **explicit precedence rules**: -1. **Environment Variables** (highest priority) -2. **`.env` file** in project root -3. **`data/config.json`** file -4. **Default values** (lowest priority) +1. **Environment Variables** (highest priority) - Takes precedence over all other sources +2. **`.env` file** in project root - Loaded as environment variables +3. **`data/config.json`** file - Persistent file-based configuration +4. **Default values** (lowest priority) - Built-in fallback values + +### Precedence Rules + +**Critical Principle**: `ENV VARS > config.json > defaults` + +- **Environment variables always win**: If a value is set via environment variable, it will NOT be overridden by config.json +- **config.json as fallback**: If an ENV var is not set (or is empty/default), the value from config.json is used +- **Defaults as last resort**: Built-in default values are used only if neither ENV var nor config.json provide a value ### Loading Mechanism -Configuration is loaded at application startup via Pydantic Settings. +Configuration is loaded at application startup in `src/server/fastapi_app.py`: -```python -# src/config/settings.py -class Settings(BaseSettings): - model_config = SettingsConfigDict(env_file=".env", extra="ignore") +1. **Pydantic Settings** loads ENV vars and .env file with defaults +2. **config.json** is loaded via `ConfigService` +3. **Selective sync**: config.json values sync to settings **only if** ENV var not set +4. **Runtime access**: Code uses `settings` object (which has final merged values) + +**Example**: +```bash +# If ENV var is set: +ANIME_DIRECTORY=/env/path # This takes precedence + +# config.json has: +{"other": {"anime_directory": "/config/path"}} # This is ignored + +# Result: settings.anime_directory = "/env/path" ``` -Source: [src/config/settings.py](../src/config/settings.py#L1-L96) +**Source**: [src/config/settings.py](../src/config/settings.py#L1-L96), [src/server/fastapi_app.py](../src/server/fastapi_app.py#L139-L185) --- diff --git a/docs/instructions.md b/docs/instructions.md index 630a912..58b3780 100644 --- a/docs/instructions.md +++ b/docs/instructions.md @@ -224,16 +224,26 @@ For each task completed: ### Architecture Review Findings - Medium Priority Issues -#### Issue 9: Configuration Scattered Across Multiple Sources +#### Issue 9: Configuration Scattered Across Multiple Sources ✅ RESOLVED - **Locations**: - - `src/config/settings.py` (environment-based settings) - - `data/config.json` (file-based configuration) - - Hardcoded values in multiple service files -- **Problem**: Configuration access is inconsistent - unclear source of truth -- **Impact**: Difficult to trace where values come from, testing complexity -- **Fix Required**: Single configuration source with clear precedence rules -- **Severity**: MEDIUM - Maintenance burden + - ✅ `src/config/settings.py` (environment-based settings) - PRIMARY source + - ✅ `data/config.json` (file-based configuration) - SECONDARY source + - ✅ Precedence now clearly defined and enforced +- **Problem**: Configuration access was inconsistent with unclear source of truth and precedence +- **Impact**: Difficult to trace where values come from, testing complexity, ENV vars being overridden +- **Fix Applied**: + - **Precedence Established**: ENV vars > config.json > defaults (explicitly enforced) + - Updated `fastapi_app.py` to only sync config.json values if ENV var not set (respects precedence) + - Added precedence logging to show which source is used + - Documented precedence rules with examples in `CONFIGURATION.md` + - **Principle**: ENV variables always take precedence, config.json is fallback only +- **Resolution Date**: January 24, 2026 +- **Files Modified**: + - `src/server/fastapi_app.py` - Implemented selective sync with precedence checks + - `docs/CONFIGURATION.md` - Documented precedence rules and examples +- **Architecture Decision**: Settings object (`settings.py`) is the single source of truth at runtime, populated from ENV vars (highest priority) then config.json (fallback) +- **Severity**: MEDIUM - Maintenance burden (FIXED) #### Issue 10: Inconsistent Error Handling Patterns diff --git a/src/server/fastapi_app.py b/src/server/fastapi_app.py index 7ae4031..0cd91f2 100644 --- a/src/server/fastapi_app.py +++ b/src/server/fastapi_app.py @@ -137,6 +137,8 @@ async def lifespan(_application: FastAPI): raise # Database is required, fail startup if it fails # Load configuration from config.json and sync with settings + # Precedence: ENV vars > config.json > defaults + # Only sync from config.json if setting is at default value try: from src.server.services.config_service import get_config_service config_service = get_config_service() @@ -147,33 +149,46 @@ async def lifespan(_application: FastAPI): ) # Sync anime_directory from config.json to settings - # config.other is Dict[str, object] - pylint doesn't infer this + # Only if not already set via ENV var (i.e., still empty) other_settings = dict(config.other) if config.other else {} if other_settings.get("anime_directory"): anime_dir = other_settings["anime_directory"] - settings.anime_directory = str(anime_dir) - logger.info( - "Loaded anime_directory from config: %s", - settings.anime_directory - ) + # Only override if settings.anime_directory is empty (default) + if not settings.anime_directory: + settings.anime_directory = str(anime_dir) + logger.info( + "Loaded anime_directory from config.json: %s", + settings.anime_directory + ) + else: + logger.info( + "anime_directory from ENV var takes precedence: %s", + settings.anime_directory + ) else: logger.debug( "anime_directory not found in config.other" ) # Sync NFO settings from config.json to settings + # Only if not already set via ENV var if config.nfo: - if config.nfo.tmdb_api_key: + # TMDB API key: ENV takes precedence + if config.nfo.tmdb_api_key and not settings.tmdb_api_key: settings.tmdb_api_key = config.nfo.tmdb_api_key - logger.info("Loaded TMDB API key from config") + logger.info("Loaded TMDB API key from config.json") + elif settings.tmdb_api_key: + logger.info("Using TMDB API key from ENV var") + # NFO boolean flags: Sync from config.json + # (These have proper defaults, so we can sync them) settings.nfo_auto_create = config.nfo.auto_create settings.nfo_update_on_scan = config.nfo.update_on_scan settings.nfo_download_poster = config.nfo.download_poster settings.nfo_download_logo = config.nfo.download_logo settings.nfo_download_fanart = config.nfo.download_fanart settings.nfo_image_size = config.nfo.image_size - logger.debug("Synced NFO settings from config") + logger.debug("Synced NFO settings from config.json") except (OSError, ValueError, KeyError) as e: logger.warning("Failed to load config from config.json: %s", e)