From e1eea7295defc02456cd089f685c67fa530ab4eb Mon Sep 17 00:00:00 2001 From: FrederikBaerentsen Date: Sat, 6 Dec 2025 20:48:30 +0100 Subject: [PATCH] fix(env): moved .env to data folder. admin page, now correctly works with changes to variables --- .env.sample | 20 +++ CHANGELOG.md | 21 +++- bricktracker/app.py | 41 ++++++ bricktracker/config_manager.py | 15 ++- bricktracker/views/admin/admin.py | 60 ++++++++- docs/migration_guide.md | 78 +++++++++++- templates/admin/configuration.html | 194 ++++++++++++++++++----------- 7 files changed, 341 insertions(+), 88 deletions(-) diff --git a/.env.sample b/.env.sample index 3c04b7c..62abf85 100644 --- a/.env.sample +++ b/.env.sample @@ -1,3 +1,23 @@ +# ================================================================================================ +# BrickTracker Configuration File +# ================================================================================================ +# +# FILE LOCATION (v1.3+): +# ---------------------- +# This file can be placed in two locations: +# 1. data/.env (RECOMMENDED) - Included in data volume backup, settings persist via admin panel +# 2. .env (root) - Backward compatible +# +# Priority: data/.env > .env (root) +# +# The application automatically detects and uses the correct location at runtime. +# +# For Docker: +# - Recommended: Place this file as data/.env (included in data volume) +# - Backward compatible: Keep as .env in root (add "env_file: .env" to compose.yaml) +# +# ================================================================================================ +# # Note on *_DEFAULT_ORDER # If set, it will append a direct ORDER BY to the SQL query # while listing objects. You can look at the structure of the SQLite database to diff --git a/CHANGELOG.md b/CHANGELOG.md index 6251b20..52747b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,18 +13,25 @@ - **New default paths** (automatically used for new installations): - Database: `data/app.db` (was: `app.db` in root) + - Configuration: `data/.env` (was: `.env` in root) - *optional, backward compatible* - CSV files: `data/*.csv` (was: `*.csv` in root) - Images/PDFs: `data/{sets,parts,minifigures,instructions}/` (was: `static/*`) +- **Configuration file (.env) location**: + - New recommended location: `data/.env` (included in data volume, settings persist) + - Backward compatible: `.env` in root still works (requires volume mount for admin panel persistence) + - Priority: `data/.env` > `.env` (automatic detection, no migration required) + - **Migration options**: - 1. **Migrate to new structure** - 2. **Keep current setup** + 1. **Migrate to new structure** (recommended - single volume for all data including .env) + 2. **Keep current setup** (backward compatible - old paths continue to work) See [Migration Guide](docs/migration_guide.md) for detailed instructions - **Docker users**: - New setup uses single volume: `data:/app/data/` (replaces 5 separate volumes) - - Old volume mounts still work if you set environment variables above + - Configuration can be in `data/.env` (recommended) or `.env` (backward compatible) + - Old volume mounts still work if you keep environment variables unchanged #### Features @@ -157,6 +164,14 @@ See [Migration Guide](docs/migration_guide.md) for detailed instructions - Ensures all-or-nothing behavior: if any part fails (set info, parts, minifigs), nothing is committed - Prevents partial set additions that would leave the database in an inconsistent state - Metadata updates (owners, tags) now defer until final commit +- Admin panel configuration loading for data/.env + - Application now loads data/.env into environment at startup + - Admin panel correctly displays current values from data/.env + - Environment variables from Docker take precedence over .env file values +- Add configuration persistence warning in admin panel + - Warning banner shows when using .env in root (non-persistent) + - Success banner shows when using data/.env (persistent) + - Provides migration instructions directly in the UI ### 1.2.4 diff --git a/bricktracker/app.py b/bricktracker/app.py index 649fabf..228ec6b 100644 --- a/bricktracker/app.py +++ b/bricktracker/app.py @@ -1,6 +1,8 @@ import logging +import os import sys import time +from pathlib import Path from zoneinfo import ZoneInfo from flask import current_app, Flask, g @@ -38,7 +40,46 @@ from bricktracker.views.storage import storage_page from bricktracker.views.wish import wish_page +def load_env_file() -> None: + """Load .env file into os.environ with priority: data/.env > .env (root)""" + data_env = Path('data/.env') + root_env = Path('.env') + + env_file = None + if data_env.exists(): + env_file = data_env + logging.info(f"Loading environment from: {data_env}") + elif root_env.exists(): + env_file = root_env + logging.info(f"Loading environment from: {root_env} (consider migrating to data/.env)") + + if env_file: + # Simple .env parser (no external dependencies needed) + with open(env_file, 'r', encoding='utf-8') as f: + for line in f: + line = line.strip() + # Skip comments and empty lines + if not line or line.startswith('#'): + continue + # Parse key=value + if '=' in line: + key, value = line.split('=', 1) + key = key.strip() + value = value.strip() + # Remove quotes if present + if value.startswith('"') and value.endswith('"'): + value = value[1:-1] + elif value.startswith("'") and value.endswith("'"): + value = value[1:-1] + # Only set if not already in environment (environment variables take precedence) + if key not in os.environ: + os.environ[key] = value + + def setup_app(app: Flask) -> None: + # Load .env file before configuration (if not already loaded by Docker Compose) + load_env_file() + # Load the configuration BrickConfigurationList(app) diff --git a/bricktracker/config_manager.py b/bricktracker/config_manager.py index 2871d0d..1a2bb46 100644 --- a/bricktracker/config_manager.py +++ b/bricktracker/config_manager.py @@ -107,7 +107,20 @@ class ConfigManager: """Manages live configuration updates for BrickTracker""" def __init__(self): - self.env_file_path = Path('.env') + # Check for .env in data folder first (v1.3+), fallback to root (backward compatibility) + data_env = Path('data/.env') + root_env = Path('.env') + + if data_env.exists(): + self.env_file_path = data_env + logger.info("Using configuration file: data/.env") + elif root_env.exists(): + self.env_file_path = root_env + logger.info("Using configuration file: .env (consider migrating to data/.env)") + else: + # Default to data/.env for new installations + self.env_file_path = data_env + logger.info("Configuration file will be created at: data/.env") def get_current_config(self) -> Dict[str, Any]: """Get current configuration values for live-changeable variables""" diff --git a/bricktracker/views/admin/admin.py b/bricktracker/views/admin/admin.py index 99a940d..f3dbd99 100644 --- a/bricktracker/views/admin/admin.py +++ b/bricktracker/views/admin/admin.py @@ -1,4 +1,5 @@ import logging +import os from flask import Blueprint, request, render_template, current_app, jsonify from flask_login import login_required @@ -31,22 +32,33 @@ admin_page = Blueprint('admin', __name__, url_prefix='/admin') def get_env_values(): """Get current environment values, using defaults from config when not set""" - import os from pathlib import Path env_values = {} config_defaults = {} env_explicit_values = {} # Track which values are explicitly set + env_locked_values = {} # Track which values are set via Docker environment (locked) + + # Read .env file if it exists (check both locations) + env_file = None + if Path('data/.env').exists(): + env_file = Path('data/.env') + elif Path('.env').exists(): + env_file = Path('.env') - # Read .env file if it exists - env_file = Path('.env') env_from_file = {} - if env_file.exists(): + if env_file: with open(env_file, 'r', encoding='utf-8') as f: for line in f: line = line.strip() if line and not line.startswith('#') and '=' in line: key, value = line.split('=', 1) + # Strip quotes from value when reading + value = value.strip() + if value.startswith('"') and value.endswith('"'): + value = value[1:-1] + elif value.startswith("'") and value.endswith("'"): + value = value[1:-1] env_from_file[key] = value # Process each config item @@ -64,6 +76,19 @@ def get_env_values(): # For int/other types, keep the original default value config_defaults[env_name] = default_value + # Check if value is set via Docker environment and overrides .env file + # A variable is "locked" if: + # 1. It's set in os.environ (Docker environment), AND + # 2. Either it's NOT in .env file, OR the value differs from .env file + is_locked = False + if env_name in os.environ: + env_value = os.environ[env_name] + file_value = env_from_file.get(env_name) + # Locked if not in file, or values differ + if file_value is None or env_value != file_value: + is_locked = True + env_locked_values[env_name] = is_locked + # Check if value is explicitly set in .env file or environment is_explicitly_set = env_name in env_from_file or env_name in os.environ env_explicit_values[env_name] = is_explicitly_set @@ -88,7 +113,7 @@ def get_env_values(): env_values[env_name] = value - return env_values, config_defaults, env_explicit_values + return env_values, config_defaults, env_explicit_values, env_locked_values # Admin @@ -202,13 +227,36 @@ def admin() -> str: open_tag ) - env_values, config_defaults, env_explicit_values = get_env_values() + env_values, config_defaults, env_explicit_values, env_locked_values = get_env_values() + + # Check .env file location and set warnings + env_file_location = None + env_file_warning = False + env_file_missing = False + + if os.path.exists('data/.env'): + env_file_location = 'data/.env' + env_file_warning = False + env_file_missing = False + elif os.path.exists('.env'): + env_file_location = '.env' + env_file_warning = True # Warn: changes won't persist without volume mount + env_file_missing = False + else: + env_file_location = None + env_file_warning = False + env_file_missing = True # Warn: no .env file found + return render_template( 'admin.html', configuration=BrickConfigurationList.list(), env_values=env_values, config_defaults=config_defaults, env_explicit_values=env_explicit_values, + env_locked_values=env_locked_values, + env_file_location=env_file_location, + env_file_warning=env_file_warning, + env_file_missing=env_file_missing, database_counters=database_counters, database_error=request.args.get('database_error'), database_exception=database_exception, diff --git a/docs/migration_guide.md b/docs/migration_guide.md index 1a2c34e..878314e 100644 --- a/docs/migration_guide.md +++ b/docs/migration_guide.md @@ -21,6 +21,7 @@ For example: `data/app.db` → `/app/data/app.db` ``` Container (/app/): ├── data/ # NEW: Single volume mount for all user data +│ ├── .env # Configuration (recommended location) │ ├── app.db # Database │ ├── retired_sets.csv # Downloaded CSV files │ ├── themes.csv @@ -87,10 +88,13 @@ This is the recommended approach for cleaner backups and simpler bind mount mana Run: ```bash + # Move configuration file (optional but recommended) + mv .env ./bricktracker-data/.env + # Move database and CSV files mv ./data/app.db ./bricktracker-data/ mv ./data/retired_sets.csv ./bricktracker-data/ - mv ./data/themes.csv ./bricktracker-data/ + mv ./data/themes.csv ./bricktracker-data/ # Move image and instruction folders mv ./instructions/* ./bricktracker-data/instructions/ @@ -106,10 +110,10 @@ This is the recommended approach for cleaner backups and simpler bind mount mana volumes: - ./bricktracker-data:/app/data/ - # Remove old volume mounts + # Remove old volume mounts and env_file (if .env was moved to data/) ``` -5. **Remove old environment overrides from `.env` (if present):** +5. **Remove old path overrides from `.env` (if present):** Delete any lines starting with: - `BK_DATABASE_PATH=` - `BK_INSTRUCTIONS_FOLDER=` @@ -173,6 +177,45 @@ If you want to keep your current volume structure without moving any files: That's it! Your existing setup will continue to work. +## Configuration File (.env) Location + +### New Behavior (v1.3+) + +BrickTracker now supports `.env` in two locations with automatic detection: + +1. **data/.env** (recommended - new location) + - Included in data volume backup + - Settings persist when changed via admin panel + - Priority location (checked first) + - **No `env_file` needed** - app reads it directly from `/app/data/.env` + +2. **.env** (backward compatibility - root) + - Continues to work for existing installations + - Requires `env_file: .env` in compose.yaml for Docker to load it at startup + - Not included in data volume (unless you add `.env` to `data/`) + +### Migration Steps for .env + +**Option A: Move to data folder (recommended)** + +```bash +# Move .env to data folder +mv .env data/.env + +# Update compose.yaml - remove or comment out env_file +# The app will automatically find and use /app/data/.env +``` + +**Option B: Keep in root (backward compatible)** + +```bash +# No changes needed +# Keep env_file: .env in compose.yaml +# App will use .env from root as fallback +``` + +**Note:** The application automatically detects which location has the .env file at runtime. No Docker Compose `env_file` directive is needed for `data/.env` because the app reads it directly from the mounted volume. + ## Configuration Reference ### New Default Paths (v1.3+) @@ -257,6 +300,27 @@ To preserve old volume structure without migration, add to `.env`: # Should show: app.db, retired_sets.csv, themes.csv, and subdirectories ``` +### Settings Don't Persist After Restart + +**Error:** Admin panel changes revert after `docker compose restart` + +**Solution:** + +This happens when `.env` is not in a volume. Choose one: + +**Option A: Move .env to data folder** +```bash +mv .env data/.env +# Update compose.yaml - remove or comment out env_file +# The app will automatically find and use /app/data/.env +``` + +**Option B: Mount .env as volume** +```yaml +volumes: + - ./.env:/app/.env +``` + ### Permission Errors If you see permission errors after migration: @@ -266,6 +330,14 @@ If you see permission errors after migration: sudo chown -R $(id -u):$(id -g) ./bricktracker-data ``` +**Permission denied writing .env:** + +If the admin panel shows an error when saving settings: + +1. Ensure .env file is writable by container user +2. If using volume mount, check host file permissions +3. In container: `docker exec BrickTracker ls -la /app/.env` or `/app/data/.env` + ### Reverting Migration If you need to revert to the old structure: diff --git a/templates/admin/configuration.html b/templates/admin/configuration.html index 405c4ec..ea5e716 100644 --- a/templates/admin/configuration.html +++ b/templates/admin/configuration.html @@ -1,10 +1,21 @@ {% import 'macro/accordion.html' as accordion %} + +{% macro is_locked(var_name) %}{{ 'disabled' if env_locked_values[var_name] else '' }}{% endmacro %} + {% macro config_badges(var_name) %} {% set current_value = env_values[var_name] %} {% set default_value = config_defaults[var_name] %} {% set is_explicitly_set = env_explicit_values[var_name] %} + {% set is_locked = env_locked_values[var_name] %} + + + {% if is_locked %} + + Locked + + {% endif %} {% if not is_explicitly_set %} @@ -24,7 +35,7 @@ {% endif %} - {% if current_value != default_value %} + {% if current_value != default_value and not is_locked %} Changed {% endif %} {% endmacro %} @@ -34,6 +45,38 @@ + +{% if env_file_warning %} +
+
Configuration Persistence Warning
+

+ Configuration file location: {{ env_file_location }} +

+

+ + Settings changes will not persist across container restarts. + To persist configuration changes, either: +

    +
  • Move .env to data/.env folder (recommended), or
    • +
  • Add volume mount: - ./.env:/app/.env to your compose.yaml
    • +
+ See Migration Guide for details. + +

+
+{% elif env_file_missing %} +
+
No Configuration File Found
+

+ + No .env file was found. Currently using default values and Docker environment variables. + If you save changes, a new configuration file will be created at data/.env (recommended location). + See Migration Guide for details. + +

+
+{% endif %} +
Badge Legend
@@ -42,7 +85,8 @@ True Boolean setting enabled
False Boolean setting disabled
- Set Custom value configured + Set Custom value configured
+ Locked Set via Docker environment (cannot be changed)
@@ -81,7 +125,7 @@
- +
- +
- +
- +
- +
- +