mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-20 18:51:26 -03:00
feat(settings): add an explicit opt-out from persisted portable mode
Setting LORA_MANAGER_PORTABLE=1 once wrote use_portable_settings: true into the plugin's own settings.json, and every later run of every instance sharing that plugin folder then read and wrote the portable settings directory. There was no way back except editing the file by hand, which is exactly the trap a user hit while following the FAQ's instructions for isolating a second instance (#1114). LORA_MANAGER_PORTABLE=0 is now the explicit exit: - _should_use_portable_settings honours "0" as a forced off, so the resolved settings directory no longer depends on the persisted flag. - SettingsManager clears the persisted flag in that case, so later runs without the variable stay on the shared settings directory. Unset or unrecognised values keep the previous behaviour: the persisted flag decides, so existing portable installs are unaffected. LORA_MANAGER_SETTINGS_DIR still takes precedence over both.
This commit is contained in:
@@ -37,6 +37,7 @@ from ..utils.constants import (
|
||||
from ..utils.preview_selection import VALID_MATURE_BLUR_LEVELS
|
||||
from ..utils.settings_paths import (
|
||||
APP_NAME,
|
||||
_portable_env_override,
|
||||
ensure_settings_file,
|
||||
get_legacy_settings_path,
|
||||
get_settings_dir_override,
|
||||
@@ -172,13 +173,23 @@ class SettingsManager:
|
||||
self._check_environment_variables()
|
||||
self._collect_configuration_warnings()
|
||||
|
||||
if (
|
||||
os.environ.get("LORA_MANAGER_PORTABLE", "0") == "1"
|
||||
and not is_settings_dir_pinned()
|
||||
):
|
||||
portable_override = _portable_env_override()
|
||||
if portable_override is True and not is_settings_dir_pinned():
|
||||
if not self.settings.get("use_portable_settings"):
|
||||
self.settings["use_portable_settings"] = True
|
||||
self._save_settings()
|
||||
elif portable_override is False and self.settings.get(
|
||||
"use_portable_settings"
|
||||
):
|
||||
# Explicit opt-out from a persisted portable mode: clear the flag so
|
||||
# later runs go back to the shared settings directory instead of
|
||||
# requiring a manual edit of settings.json.
|
||||
logger.info(
|
||||
"Clearing the persisted portable-mode flag because %s=0",
|
||||
"LORA_MANAGER_PORTABLE",
|
||||
)
|
||||
self.settings["use_portable_settings"] = False
|
||||
self._save_settings()
|
||||
|
||||
if self._needs_initial_save:
|
||||
self._save_settings()
|
||||
|
||||
@@ -174,12 +174,42 @@ def ensure_settings_file(logger: Optional[logging.Logger] = None) -> str:
|
||||
return target_path
|
||||
|
||||
|
||||
def _portable_env_override() -> Optional[bool]:
|
||||
"""Return the portable mode forced by ``LORA_MANAGER_PORTABLE``, if any.
|
||||
|
||||
Returns:
|
||||
``True`` when the variable enables portable mode, ``False`` when it is
|
||||
explicitly set to ``"0"``, and ``None`` when it is unset or holds some
|
||||
other value (in which case the persisted settings flag decides).
|
||||
"""
|
||||
|
||||
raw = os.environ.get(_LM_PORTABLE_ENV)
|
||||
if raw is None:
|
||||
return None
|
||||
if raw == "1":
|
||||
return True
|
||||
if raw == "0":
|
||||
return False
|
||||
return None
|
||||
|
||||
|
||||
def _should_use_portable_settings(path: str, logger: logging.Logger) -> bool:
|
||||
"""Return ``True`` when the env var forces it or the settings file enables it."""
|
||||
|
||||
if os.environ.get(_LM_PORTABLE_ENV, "0") == "1":
|
||||
override = _portable_env_override()
|
||||
if override is True:
|
||||
logger.debug("Portable mode enabled via %s", _LM_PORTABLE_ENV)
|
||||
return True
|
||||
if override is False:
|
||||
# Explicit opt-out. Without this, a single `LORA_MANAGER_PORTABLE=1`
|
||||
# run would pin the shared plugin settings.json to portable mode
|
||||
# forever, with no way back except editing that file by hand.
|
||||
logger.info(
|
||||
"Portable mode disabled via %s=%s",
|
||||
_LM_PORTABLE_ENV,
|
||||
os.environ.get(_LM_PORTABLE_ENV, ""),
|
||||
)
|
||||
return False
|
||||
|
||||
if not os.path.exists(path):
|
||||
return False
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
"""Tests for the portable-mode flag lifecycle (issue #1114 follow-up).
|
||||
|
||||
``LORA_MANAGER_PORTABLE=1`` persists ``use_portable_settings: true`` into the
|
||||
plugin''s own settings.json. That is convenient for repeat runs, but it used to
|
||||
be a one-way trip: the flag made every instance sharing that plugin folder read
|
||||
(and write) the portable settings directory, and the only way back was editing
|
||||
settings.json by hand. ``LORA_MANAGER_PORTABLE=0`` is now the explicit exit.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from py.services import settings_manager as settings_manager_module
|
||||
from py.services.settings_manager import SettingsManager
|
||||
|
||||
|
||||
def _write_settings(path, **extra):
|
||||
payload = {
|
||||
"folder_paths": {"loras": ["/loras"]},
|
||||
}
|
||||
payload.update(extra)
|
||||
path.write_text(json.dumps(payload), encoding="utf-8")
|
||||
return payload
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def isolated_settings_path(tmp_path, monkeypatch):
|
||||
"""Point SettingsManager at a settings.json we control."""
|
||||
settings_path = tmp_path / "settings.json"
|
||||
monkeypatch.setattr(
|
||||
"py.services.settings_manager.ensure_settings_file",
|
||||
lambda logger=None: str(settings_path),
|
||||
)
|
||||
settings_manager_module.reset_settings_manager()
|
||||
yield settings_path
|
||||
settings_manager_module.reset_settings_manager()
|
||||
|
||||
|
||||
def test_portable_env_enables_and_persists_the_flag(
|
||||
isolated_settings_path, monkeypatch
|
||||
):
|
||||
_write_settings(isolated_settings_path)
|
||||
monkeypatch.setenv("LORA_MANAGER_PORTABLE", "1")
|
||||
|
||||
manager = SettingsManager()
|
||||
|
||||
assert manager.get("use_portable_settings") is True
|
||||
persisted = json.loads(isolated_settings_path.read_text(encoding="utf-8"))
|
||||
assert persisted["use_portable_settings"] is True
|
||||
|
||||
|
||||
def test_explicit_zero_clears_the_persisted_flag(
|
||||
isolated_settings_path, monkeypatch
|
||||
):
|
||||
"""`=0` must undo a previous `=1`, without hand-editing settings.json."""
|
||||
_write_settings(isolated_settings_path, use_portable_settings=True)
|
||||
monkeypatch.setenv("LORA_MANAGER_PORTABLE", "0")
|
||||
|
||||
manager = SettingsManager()
|
||||
|
||||
assert manager.get("use_portable_settings") is False
|
||||
persisted = json.loads(isolated_settings_path.read_text(encoding="utf-8"))
|
||||
# A default value is omitted from disk, so the key is gone entirely.
|
||||
assert persisted.get("use_portable_settings") is None
|
||||
|
||||
|
||||
def test_unset_env_keeps_the_persisted_flag(
|
||||
isolated_settings_path, monkeypatch
|
||||
):
|
||||
"""Portable mode must persist across runs when the variable is unset."""
|
||||
_write_settings(isolated_settings_path, use_portable_settings=True)
|
||||
monkeypatch.delenv("LORA_MANAGER_PORTABLE", raising=False)
|
||||
|
||||
manager = SettingsManager()
|
||||
|
||||
assert manager.get("use_portable_settings") is True
|
||||
|
||||
|
||||
def test_zero_is_a_noop_when_portable_was_never_enabled(
|
||||
isolated_settings_path, monkeypatch
|
||||
):
|
||||
_write_settings(isolated_settings_path)
|
||||
monkeypatch.setenv("LORA_MANAGER_PORTABLE", "0")
|
||||
|
||||
manager = SettingsManager()
|
||||
|
||||
assert manager.get("use_portable_settings") in (False, None)
|
||||
|
||||
|
||||
def test_pinned_settings_dir_wins_over_portable_env(
|
||||
isolated_settings_path, monkeypatch
|
||||
):
|
||||
"""LORA_MANAGER_SETTINGS_DIR still takes precedence, as documented."""
|
||||
_write_settings(isolated_settings_path)
|
||||
monkeypatch.setenv("LORA_MANAGER_PORTABLE", "1")
|
||||
monkeypatch.setenv("LORA_MANAGER_SETTINGS_DIR", str(isolated_settings_path.parent))
|
||||
|
||||
manager = SettingsManager()
|
||||
|
||||
# The pinned directory already decides the location, so the portable flag
|
||||
# is deliberately left alone.
|
||||
assert not manager.get("use_portable_settings")
|
||||
@@ -34,10 +34,12 @@ class TestShouldUsePortableSettings:
|
||||
@pytest.mark.parametrize(
|
||||
"env_value, settings_flag, expected",
|
||||
[
|
||||
("1", False, True), # env = 1 overrides settings.json false
|
||||
("1", False, True), # env = 1 forces portable on
|
||||
("1", True, True), # env = 1 matches settings.json true
|
||||
("0", False, False), # env = 0 → rely on settings.json
|
||||
("0", True, True), # env = 0 → rely on settings.json
|
||||
("0", False, False), # env = 0 forces portable off
|
||||
("0", True, False), # env = 0 overrides a persisted true
|
||||
("yes", False, False), # unrecognised value → rely on settings.json
|
||||
("yes", True, True), # unrecognised value → rely on settings.json
|
||||
("", False, False), # unset → rely on settings.json
|
||||
("", True, True), # unset → rely on settings.json
|
||||
],
|
||||
@@ -58,6 +60,21 @@ class TestShouldUsePortableSettings:
|
||||
result = _should_use_portable_settings(str(settings_file), logging.getLogger())
|
||||
assert result == expected
|
||||
|
||||
def test_explicit_zero_is_the_documented_opt_out(self, tmp_path, caplog):
|
||||
"""`=0` must be honoured even against a persisted true flag."""
|
||||
settings_file = tmp_path / "settings.json"
|
||||
settings_file.write_text(json.dumps({"use_portable_settings": True}))
|
||||
|
||||
with pytest.MonkeyPatch.context() as mp:
|
||||
mp.setenv("LORA_MANAGER_PORTABLE", "0")
|
||||
with caplog.at_level(logging.INFO):
|
||||
result = _should_use_portable_settings(
|
||||
str(settings_file), logging.getLogger()
|
||||
)
|
||||
|
||||
assert result is False
|
||||
assert "Portable mode disabled" in caplog.text
|
||||
|
||||
def test_missing_file_without_env(self, tmp_path):
|
||||
"""Without env var, missing settings file returns False."""
|
||||
missing = tmp_path / "nonexistent.json"
|
||||
|
||||
Reference in New Issue
Block a user