From b5ee4a640860f79e3cedfdf33b58564a93e74c77 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sun, 26 Oct 2025 18:06:37 +0800 Subject: [PATCH] feat(settings): enhance settings handling and add startup messages, fixes #593 and fixes #594 - Add standalone mode detection via LORA_MANAGER_STANDALONE environment variable - Improve error handling for settings file loading with specific JSON decode errors - Add startup messages system to communicate configuration warnings and errors to users - Include settings file path and startup messages in settings API response - Automatically save settings when bootstrapping from defaults due to missing/invalid settings file - Add configuration warnings collection for environment variables and other settings issues The changes improve robustness of settings management and provide better user feedback when configuration issues occur. --- py/routes/handlers/misc_handlers.py | 11 +- py/services/settings_manager.py | 132 ++++++++++++++++++++- standalone.py | 83 ++++++------- static/js/managers/SettingsManager.js | 102 +++++++++++++++- tests/standalone/test_standalone_server.py | 5 +- tests/test_standalone_settings.py | 93 +++++++++++++++ 6 files changed, 372 insertions(+), 54 deletions(-) create mode 100644 tests/test_standalone_settings.py diff --git a/py/routes/handlers/misc_handlers.py b/py/routes/handlers/misc_handlers.py index 638689dc..dcd91f95 100644 --- a/py/routes/handlers/misc_handlers.py +++ b/py/routes/handlers/misc_handlers.py @@ -238,7 +238,16 @@ class SettingsHandler: value = self._settings.get(key) if value is not None: response_data[key] = value - return web.json_response({"success": True, "settings": response_data}) + settings_file = getattr(self._settings, "settings_file", None) + if settings_file: + response_data["settings_file"] = settings_file + messages_getter = getattr(self._settings, "get_startup_messages", None) + messages = list(messages_getter()) if callable(messages_getter) else [] + return web.json_response({ + "success": True, + "settings": response_data, + "messages": messages, + }) except Exception as exc: # pragma: no cover - defensive logging logger.error("Error getting settings: %s", exc, exc_info=True) return web.json_response({"success": False, "error": str(exc)}, status=500) diff --git a/py/services/settings_manager.py b/py/services/settings_manager.py index 1094037a..e714c219 100644 --- a/py/services/settings_manager.py +++ b/py/services/settings_manager.py @@ -53,6 +53,10 @@ DEFAULT_SETTINGS: Dict[str, Any] = { class SettingsManager: def __init__(self): self.settings_file = ensure_settings_file(logger) + self._standalone_mode = self._detect_standalone_mode() + self._startup_messages: List[Dict[str, Any]] = [] + self._needs_initial_save = False + self._bootstrap_reason: Optional[str] = None self.settings = self._load_settings() self._migrate_setting_keys() self._ensure_default_settings() @@ -60,6 +64,16 @@ class SettingsManager: self._migrate_download_path_template() self._auto_set_default_roots() self._check_environment_variables() + self._collect_configuration_warnings() + + if self._needs_initial_save: + self._save_settings() + self._needs_initial_save = False + + def _detect_standalone_mode(self) -> bool: + """Return ``True`` when running in standalone mode.""" + + return os.environ.get("LORA_MANAGER_STANDALONE") == "1" def _load_settings(self) -> Dict[str, Any]: """Load settings from file""" @@ -67,8 +81,39 @@ class SettingsManager: try: with open(self.settings_file, 'r', encoding='utf-8') as f: return json.load(f) - except Exception as e: - logger.error(f"Error loading settings: {e}") + except json.JSONDecodeError as exc: + logger.error("Failed to parse settings.json: %s", exc) + self._add_startup_message( + code="settings-json-invalid", + title="Settings file could not be parsed", + message=( + "LoRA Manager could not parse settings.json. Default settings " + "will be used for this session." + ), + severity="error", + actions=self._default_settings_actions(), + details=str(exc), + dismissible=False, + ) + self._needs_initial_save = True + self._bootstrap_reason = "invalid" + except Exception as exc: # pragma: no cover - defensive guard + logger.error("Unexpected error loading settings: %s", exc) + self._add_startup_message( + code="settings-json-unreadable", + title="Settings file could not be read", + message="LoRA Manager could not read settings.json. Default settings will be used for this session.", + severity="error", + actions=self._default_settings_actions(), + details=str(exc), + dismissible=False, + ) + self._needs_initial_save = True + self._bootstrap_reason = "unreadable" + + if not os.path.exists(self.settings_file): + self._needs_initial_save = True + self._bootstrap_reason = "missing" return self._get_default_settings() def _ensure_default_settings(self) -> None: @@ -393,6 +438,86 @@ class SettingsManager: self.settings['civitai_api_key'] = env_api_key self._save_settings() + def _default_settings_actions(self) -> List[Dict[str, Any]]: + return [ + { + "action": "open-settings-location", + "label": "Open settings folder", + "type": "primary", + "icon": "fas fa-folder-open", + } + ] + + def _add_startup_message( + self, + *, + code: str, + title: str, + message: str, + severity: str = "info", + actions: Optional[List[Dict[str, Any]]] = None, + details: Optional[str] = None, + dismissible: bool = False, + ) -> None: + if any(existing.get("code") == code for existing in self._startup_messages): + return + + payload: Dict[str, Any] = { + "code": code, + "title": title, + "message": message, + "severity": severity.lower(), + "dismissible": bool(dismissible), + } + + if actions: + payload["actions"] = [dict(action) for action in actions] + if details: + payload["details"] = details + payload["settings_file"] = self.settings_file + + self._startup_messages.append(payload) + + def _collect_configuration_warnings(self) -> None: + if not self._standalone_mode: + return + + folder_paths = self.settings.get('folder_paths', {}) or {} + monitored_keys = ('loras', 'checkpoints', 'embeddings') + + has_valid_paths = False + for key in monitored_keys: + raw_paths = folder_paths.get(key) or [] + if isinstance(raw_paths, str): + raw_paths = [raw_paths] + try: + iterator = list(raw_paths) + except TypeError: + continue + if any(isinstance(path, str) and path and os.path.exists(path) for path in iterator): + has_valid_paths = True + break + + if not has_valid_paths: + if self._bootstrap_reason == "missing": + message = ( + "LoRA Manager created a default settings.json because no configuration was found. " + "Edit settings.json to add your model directories so library scanning can run." + ) + else: + message = ( + "LoRA Manager could not locate any configured model directories. " + "Edit settings.json to add your model folders so library scanning can run." + ) + self._add_startup_message( + code="missing-model-paths", + title="Model folders need setup", + message=message, + severity="warning", + actions=self._default_settings_actions(), + dismissible=False, + ) + def refresh_environment_variables(self) -> None: """Refresh settings from environment variables""" self._check_environment_variables() @@ -427,6 +552,9 @@ class SettingsManager: self._save_settings() return normalized.copy() + def get_startup_messages(self) -> List[Dict[str, Any]]: + return [message.copy() for message in self._startup_messages] + def get_priority_tag_entries(self, model_type: str) -> List[PriorityTagEntry]: config = self.get_priority_tag_config() raw_config = config.get(model_type, "") diff --git a/standalone.py b/standalone.py index 63f9b263..61d53aba 100644 --- a/standalone.py +++ b/standalone.py @@ -2,7 +2,7 @@ import os import sys import json from py.middleware.cache_middleware import cache_control -from py.utils.settings_paths import ensure_settings_file, get_settings_dir +from py.utils.settings_paths import ensure_settings_file # Set environment variable to indicate standalone mode os.environ["LORA_MANAGER_STANDALONE"] = "1" @@ -228,54 +228,43 @@ class StandaloneServer: from py.lora_manager import LoraManager def validate_settings(): - """Validate that settings.json exists and has required configuration""" - settings_path = ensure_settings_file(logger) - if not os.path.exists(settings_path): - logger.error("=" * 80) - logger.error("CONFIGURATION ERROR: settings.json file not found!") - logger.error("") - logger.error("Expected location: %s", settings_path) - logger.error("") - logger.error("To run in standalone mode, you need to create a settings.json file.") - logger.error("Please follow these steps:") - logger.error("") - logger.error("1. Copy the provided settings.json.example file to create a new file") - logger.error(" named settings.json inside the LoRA Manager settings folder:") - logger.error(" %s", get_settings_dir()) - logger.error("") - logger.error("2. Edit settings.json to include your correct model folder paths") - logger.error(" and CivitAI API key") - logger.error("=" * 80) - return False - - # Check if settings.json has valid folder paths + """Initialize settings and log any startup warnings.""" try: - with open(settings_path, 'r', encoding='utf-8') as f: - settings = json.load(f) - - folder_paths = settings.get('folder_paths', {}) - has_valid_paths = False - - for path_type in ['loras', 'checkpoints', 'embeddings']: - paths = folder_paths.get(path_type, []) - if paths and any(os.path.exists(p) for p in paths): - has_valid_paths = True - break - - if not has_valid_paths: - logger.warning("=" * 80) - logger.warning("CONFIGURATION WARNING: No valid model folder paths found!") - logger.warning("") - logger.warning("Your settings.json exists but doesn't contain valid folder paths.") - logger.warning("Please check and update the folder_paths section in settings.json") - logger.warning("to include existing directories for your models.") - logger.warning("=" * 80) - return False - - except Exception as e: - logger.error(f"Error reading settings.json: {e}") + from py.services.settings_manager import get_settings_manager + + manager = get_settings_manager() + except Exception as exc: # pragma: no cover - defensive logging + logger.error("Failed to initialise settings manager: %s", exc, exc_info=True) return False - + + messages = manager.get_startup_messages() + if messages: + logger.warning("=" * 80) + logger.warning("Standalone mode is using fallback configuration values.") + for message in messages: + severity = (message.get("severity") or "info").lower() + title = message.get("title") + body = message.get("message") or "" + details = message.get("details") + location = message.get("settings_file") or manager.settings_file + + text = f"{title}: {body}" if title else body + log_method = logger.info + if severity == "error": + log_method = logger.error + elif severity == "warning": + log_method = logger.warning + + log_method(text) + if details: + log_method("Details: %s", details) + if location: + log_method("Settings file: %s", location) + + logger.warning("=" * 80) + else: + logger.info("Loaded settings from %s", manager.settings_file) + return True class StandaloneLoraManager(LoraManager): diff --git a/static/js/managers/SettingsManager.js b/static/js/managers/SettingsManager.js index fb7cbd54..0137dd65 100644 --- a/static/js/managers/SettingsManager.js +++ b/static/js/managers/SettingsManager.js @@ -7,6 +7,7 @@ import { translate } from '../utils/i18nHelpers.js'; import { i18n } from '../i18n/index.js'; import { configureModelCardVideo } from '../components/shared/ModelCard.js'; import { validatePriorityTagString, getPriorityTagSuggestionsMap, invalidatePriorityTagSuggestionsCache } from '../utils/priorityTagHelpers.js'; +import { bannerService } from './BannerService.js'; export class SettingsManager { constructor() { @@ -15,6 +16,8 @@ export class SettingsManager { this.initializationPromise = null; this.availableLibraries = {}; this.activeLibrary = ''; + this.settingsFilePath = null; + this.registeredStartupBannerIds = new Set(); // Add initialization to sync with modal state this.currentPage = document.body.dataset.page || 'loras'; @@ -52,14 +55,18 @@ export class SettingsManager { const data = await response.json(); if (data.success && data.settings) { state.global.settings = this.mergeSettingsWithDefaults(data.settings); + this.settingsFilePath = data.settings.settings_file || this.settingsFilePath; + this.registerStartupMessages(data.messages); console.log('Settings synced from backend'); } else { console.error('Failed to sync settings from backend:', data.error); state.global.settings = this.mergeSettingsWithDefaults(); + this.registerStartupMessages(data?.messages); } } catch (error) { console.error('Failed to sync settings from backend:', error); state.global.settings = this.mergeSettingsWithDefaults(); + this.registerStartupMessages(); } await this.applyLanguageSetting(); @@ -128,6 +135,90 @@ export class SettingsManager { return merged; } + registerStartupMessages(messages = []) { + if (!Array.isArray(messages) || messages.length === 0) { + return; + } + + const severityPriority = { + error: 90, + warning: 60, + info: 30, + }; + + messages.forEach((message, index) => { + if (!message || typeof message !== 'object') { + return; + } + + if (!this.settingsFilePath && typeof message.settings_file === 'string') { + this.settingsFilePath = message.settings_file; + } + + const bannerId = `startup-${message.code || index}`; + if (this.registeredStartupBannerIds.has(bannerId)) { + return; + } + + const severity = (message.severity || 'info').toLowerCase(); + const bannerTitle = message.title || 'Configuration notice'; + const bannerContent = message.message || message.content || ''; + const priority = typeof message.priority === 'number' + ? message.priority + : severityPriority[severity] || severityPriority.info; + const dismissible = message.dismissible !== false; + + const normalizedActions = Array.isArray(message.actions) + ? message.actions.map(action => ({ + text: action.label || action.text || 'Review settings', + icon: action.icon || 'fas fa-cog', + action: action.action, + type: action.type || 'primary', + url: action.url, + })) + : []; + + bannerService.registerBanner(bannerId, { + id: bannerId, + title: bannerTitle, + content: bannerContent, + actions: normalizedActions, + dismissible, + priority, + onRegister: (bannerElement) => { + normalizedActions.forEach(action => { + if (!action.action) { + return; + } + + const button = bannerElement.querySelector(`.banner-action[data-action="${action.action}"]`); + if (button) { + button.addEventListener('click', (event) => { + event.preventDefault(); + this.handleStartupBannerAction(action.action); + }); + } + }); + }, + }); + + this.registeredStartupBannerIds.add(bannerId); + }); + } + + handleStartupBannerAction(action) { + switch (action) { + case 'open-settings-modal': + modalManager.showModal('settingsModal'); + break; + case 'open-settings-location': + this.openSettingsFileLocation(); + break; + default: + console.warn('Unhandled startup banner action:', action); + } + } + // Helper method to determine if a setting should be saved to backend isBackendSetting(settingKey) { return this.backendSettingKeys.has(settingKey); @@ -199,6 +290,9 @@ export class SettingsManager { const openSettingsLocationButton = document.querySelector('.settings-open-location-trigger'); if (openSettingsLocationButton) { + if (openSettingsLocationButton.dataset.settingsPath) { + this.settingsFilePath = openSettingsLocationButton.dataset.settingsPath; + } openSettingsLocationButton.addEventListener('click', () => { const filePath = openSettingsLocationButton.dataset.settingsPath; this.openSettingsFileLocation(filePath); @@ -235,7 +329,9 @@ export class SettingsManager { } async openSettingsFileLocation(filePath) { - if (!filePath) { + const targetPath = filePath || this.settingsFilePath || document.querySelector('.settings-open-location-trigger')?.dataset.settingsPath; + + if (!targetPath) { showToast('settings.openSettingsFileLocation.failed', {}, 'error'); return; } @@ -246,13 +342,15 @@ export class SettingsManager { headers: { 'Content-Type': 'application/json', }, - body: JSON.stringify({ file_path: filePath }), + body: JSON.stringify({ file_path: targetPath }), }); if (!response.ok) { throw new Error(`Request failed with status ${response.status}`); } + this.settingsFilePath = targetPath; + showToast('settings.openSettingsFileLocation.success', {}, 'success'); } catch (error) { console.error('Failed to open settings file location:', error); diff --git a/tests/standalone/test_standalone_server.py b/tests/standalone/test_standalone_server.py index 9531bed6..999b7bf4 100644 --- a/tests/standalone/test_standalone_server.py +++ b/tests/standalone/test_standalone_server.py @@ -108,9 +108,10 @@ def test_validate_settings_warns_for_missing_model_paths(caplog, standalone_modu } ) - assert standalone_module.validate_settings() is False + assert standalone_module.validate_settings() is True warning_lines = [record.message for record in caplog.records if record.levelname == "WARNING"] - assert any("CONFIGURATION WARNING" in line for line in warning_lines) + assert any("Standalone mode is using fallback" in line for line in warning_lines) + assert any("Model folders need setup" in line for line in warning_lines) def test_standalone_lora_manager_registers_routes(monkeypatch, tmp_path, standalone_module): diff --git a/tests/test_standalone_settings.py b/tests/test_standalone_settings.py new file mode 100644 index 00000000..5c4d0c61 --- /dev/null +++ b/tests/test_standalone_settings.py @@ -0,0 +1,93 @@ +import importlib +import json +from pathlib import Path + +import pytest + +from py.services.settings_manager import get_settings_manager, reset_settings_manager +from py.utils import settings_paths + + +@pytest.fixture(autouse=True) +def reset_settings(tmp_path, monkeypatch): + """Reset the settings manager and redirect config to a temp directory.""" + def fake_user_config_dir(*args, **kwargs): + return str(tmp_path / "config") + + monkeypatch.setattr(settings_paths, "user_config_dir", fake_user_config_dir) + monkeypatch.setenv("LORA_MANAGER_STANDALONE", "1") + reset_settings_manager() + yield + reset_settings_manager() + + +def read_settings_file(path: Path) -> dict: + with path.open('r', encoding='utf-8') as handle: + return json.load(handle) + + +def test_missing_settings_creates_defaults_and_emits_warnings(tmp_path): + manager = get_settings_manager() + settings_path = Path(manager.settings_file) + + assert settings_path.exists() + assert read_settings_file(settings_path) + + messages = manager.get_startup_messages() + codes = {message["code"] for message in messages} + assert codes == {"missing-model-paths"} + + warning = messages[0] + assert "default settings.json" in warning["message"].lower() + assert warning["dismissible"] is False + + actions = warning.get("actions") or [] + assert actions == [ + { + "action": "open-settings-location", + "label": "Open settings folder", + "type": "primary", + "icon": "fas fa-folder-open", + } + ] + + +def test_invalid_settings_recovers_with_defaults(tmp_path): + config_dir = Path(settings_paths.user_config_dir()) + config_dir.mkdir(parents=True, exist_ok=True) + settings_path = config_dir / "settings.json" + settings_path.write_text("{ invalid json", encoding='utf-8') + + manager = get_settings_manager() + + assert settings_path.exists() + data = read_settings_file(settings_path) + assert isinstance(data, dict) + + codes = {message["code"] for message in manager.get_startup_messages()} + assert "settings-json-invalid" in codes + + +def test_missing_settings_skips_warning_when_embedded(tmp_path, monkeypatch): + monkeypatch.setenv("LORA_MANAGER_STANDALONE", "0") + reset_settings_manager() + + manager = get_settings_manager() + + assert manager.get_startup_messages() == [] + + +def test_validate_settings_logs_warnings(tmp_path, monkeypatch, caplog): + monkeypatch.setattr(settings_paths, "user_config_dir", lambda *args, **kwargs: str(tmp_path / "config")) + + reset_settings_manager() + import standalone + importlib.reload(standalone) + + reset_settings_manager() + + with caplog.at_level("INFO", logger="lora-manager-standalone"): + assert standalone.validate_settings() is True + + messages = [record.message for record in caplog.records] + assert any("Standalone mode is using fallback configuration values." in message for message in messages)