fix(lora): review findings for the LoraManager layout (#355)
- The example-images library subfolder goes through the folder guard like its root, so its canonical (normcased) form matches the media check on Windows for a library name with capitals. - A failed trigger-word load is retried by a running node only after TRIGGER_RETRY_SECONDS, not on every prompt run. - A review whose discovery cannot list a folder says so (folder-unreadable, the folder's name only) instead of a generic error.
This commit is contained in:
@@ -268,8 +268,10 @@ class PromptManagerBase:
|
||||
|
||||
cache = get_trigger_cache()
|
||||
|
||||
# Lazy-load cache on first use
|
||||
if not cache.is_loaded and IntegrationConfig.LORA_MANAGER_PATH:
|
||||
# Lazy-load cache on first use; a failed load waits before the next try
|
||||
# (#355: never a rescan of LoraManager's folders on every run).
|
||||
retry_due = getattr(cache, "retry_due", lambda: True)
|
||||
if not cache.is_loaded and IntegrationConfig.LORA_MANAGER_PATH and retry_due():
|
||||
cache.load(IntegrationConfig.LORA_MANAGER_PATH)
|
||||
|
||||
modified, injected = inject_trigger_words(text, cache)
|
||||
|
||||
@@ -230,6 +230,18 @@ class LoraImportRoutesMixin:
|
||||
lora_import.preview, lm_path, cache_dir, SERVABLE_EXTENSIONS
|
||||
)
|
||||
state = await self._run_in_executor(db.category_import_state, CATEGORY)
|
||||
except OSError as error:
|
||||
# A folder discovery cannot list refuses the import (it must not
|
||||
# replace the collection with part of it): say which one.
|
||||
self.logger.warning(
|
||||
f"LoRA import review: a folder could not be listed: {error}"
|
||||
)
|
||||
return _failure(
|
||||
f"A LoRA folder could not be listed ({self._public_error(error)}). "
|
||||
"Nothing was changed.",
|
||||
500,
|
||||
"folder-unreadable",
|
||||
)
|
||||
except Exception:
|
||||
self.logger.exception("LoRA import preview failed")
|
||||
return _failure("The LoRA import could not be reviewed.", 500)
|
||||
|
||||
@@ -323,8 +323,9 @@ def _example_images_root(
|
||||
root = _guard_folder(configured, home) if configured else None
|
||||
if root is None or libraries <= 1:
|
||||
return root
|
||||
scoped = os.path.join(root, _sanitize_library_name(library))
|
||||
return scoped if os.path.isdir(scoped) else None
|
||||
# Guarded (canonical) like its root: a library name keeps its case, and
|
||||
# the media check compares normcased paths.
|
||||
return _guard_folder(os.path.join(root, _sanitize_library_name(library)), home)
|
||||
|
||||
|
||||
def _media_roots(*groups) -> Tuple[str, ...]:
|
||||
|
||||
@@ -13,6 +13,7 @@ import json
|
||||
import os
|
||||
import re
|
||||
import threading
|
||||
import time
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from pathlib import Path
|
||||
@@ -826,6 +827,11 @@ def get_lora_image_cache_dir(create: bool = True) -> Path:
|
||||
_LORA_PATTERN = re.compile(r"<lora:([^:>]+):[^>]+>", re.IGNORECASE)
|
||||
|
||||
|
||||
# A failed load is retried by a running node only after this long (an explicit
|
||||
# reload, from the settings routes, always runs).
|
||||
TRIGGER_RETRY_SECONDS = 60
|
||||
|
||||
|
||||
class TriggerWordCache:
|
||||
"""Thread-safe cache mapping LoRA names to their trigger words.
|
||||
|
||||
@@ -836,6 +842,7 @@ class TriggerWordCache:
|
||||
self._cache: Dict[str, List[str]] = {}
|
||||
self._lock = threading.Lock()
|
||||
self._loaded = False
|
||||
self._failed_at: Optional[float] = None
|
||||
|
||||
def load(self, lora_manager_path: str) -> int:
|
||||
"""Build the trigger word map from the sidecars LoraManager uses.
|
||||
@@ -853,11 +860,13 @@ class TriggerWordCache:
|
||||
except Exception as error:
|
||||
logger.warning(f"Trigger word cache not refreshed: {error}")
|
||||
with self._lock:
|
||||
self._failed_at = time.monotonic()
|
||||
return len(self._cache)
|
||||
|
||||
with self._lock:
|
||||
self._cache = new_cache
|
||||
self._loaded = True
|
||||
self._failed_at = None
|
||||
|
||||
logger.info(
|
||||
f"Trigger word cache loaded: {len(new_cache)} LoRAs with trigger words"
|
||||
@@ -899,6 +908,15 @@ class TriggerWordCache:
|
||||
with self._lock:
|
||||
return self._cache.get(lora_name.lower(), [])
|
||||
|
||||
def retry_due(self) -> bool:
|
||||
"""Whether a node may try loading again: never failed, or the last
|
||||
failure is TRIGGER_RETRY_SECONDS old (no rescan on every prompt run)."""
|
||||
with self._lock:
|
||||
failed_at = self._failed_at
|
||||
return failed_at is None or time.monotonic() - failed_at >= (
|
||||
TRIGGER_RETRY_SECONDS
|
||||
)
|
||||
|
||||
@property
|
||||
def is_loaded(self) -> bool:
|
||||
with self._lock:
|
||||
@@ -908,6 +926,7 @@ class TriggerWordCache:
|
||||
with self._lock:
|
||||
self._cache.clear()
|
||||
self._loaded = False
|
||||
self._failed_at = None
|
||||
|
||||
|
||||
# Module-level singleton
|
||||
|
||||
@@ -441,6 +441,19 @@ class LoraImportRouteTests(LoraImportAPITestCase):
|
||||
[os.path.basename(i["image_path"]) for i in images], ["image_1.png"]
|
||||
)
|
||||
|
||||
async def test_an_unlistable_folder_says_so_in_the_review(self):
|
||||
self._two_loras()
|
||||
error = PermissionError(13, "Permission denied", str(self.loras / "lost+found"))
|
||||
with patch.object(lora_import, "discover_sidecars", side_effect=error):
|
||||
resp, body = await self._post(PREVIEW)
|
||||
self.assertEqual((resp.status, body["code"]), (500, "folder-unreadable"))
|
||||
self.assertEqual(
|
||||
body["error"],
|
||||
"A LoRA folder could not be listed (Permission denied: lost+found). "
|
||||
"Nothing was changed.",
|
||||
)
|
||||
self.assertNotIn(str(self.tmp_root), json.dumps(body))
|
||||
|
||||
async def test_preview_requires_the_integration(self):
|
||||
IntegrationConfig.LORA_MANAGER_ENABLED = False
|
||||
resp, body = await self._post(PREVIEW)
|
||||
|
||||
@@ -1771,6 +1771,26 @@ class TestTriggerWordCacheLayout(unittest.TestCase):
|
||||
self.assertEqual(cache.get_trigger_words("old"), ["word"])
|
||||
self.assertIn("share gone", "\n".join(logs.output))
|
||||
|
||||
def test_a_failed_load_waits_before_a_node_retries(self):
|
||||
# Review finding: a node calls load while the cache is not loaded; a
|
||||
# failing discovery must not rescan on every prompt run.
|
||||
cache = TriggerWordCache()
|
||||
self.assertTrue(cache.retry_due())
|
||||
clock = [100.0]
|
||||
with (
|
||||
patch.object(lora_utils.time, "monotonic", lambda: clock[0]),
|
||||
patch.object(lora_utils, "discover_sidecars", side_effect=OSError("x")),
|
||||
self.assertLogs("prompt_manager.lora_utils", level="WARNING"),
|
||||
):
|
||||
cache.load(str(self.lm))
|
||||
self.assertFalse(cache.is_loaded)
|
||||
self.assertFalse(cache.retry_due())
|
||||
clock[0] += lora_utils.TRIGGER_RETRY_SECONDS
|
||||
self.assertTrue(cache.retry_due())
|
||||
cache.load(str(self.lm))
|
||||
self.assertTrue(cache.is_loaded)
|
||||
self.assertTrue(cache.retry_due())
|
||||
|
||||
def test_any_other_failure_never_raises(self):
|
||||
cache = TriggerWordCache()
|
||||
with (
|
||||
|
||||
@@ -334,6 +334,24 @@ class TestLoraTriggerWords(NodeTestCase):
|
||||
self.assertEqual(text, "a cat <lora:neon:1>")
|
||||
self.assertIn("share gone", "\n".join(logs.output))
|
||||
|
||||
def test_a_failed_load_is_not_retried_on_every_run(self):
|
||||
from py import lora_utils
|
||||
|
||||
cache = lora_utils.get_trigger_cache()
|
||||
cache.clear()
|
||||
self.addCleanup(cache.clear)
|
||||
with (
|
||||
mock.patch.dict(sys.modules, {"py.config": fake_integration_config(True)}),
|
||||
mock.patch.object(
|
||||
lora_utils, "discover_sidecars", side_effect=OSError("share gone")
|
||||
) as discover,
|
||||
mock.patch.object(lora_utils, "default_lora_roots", return_value=[]),
|
||||
self.assertLogs("prompt_manager.lora_utils", level="WARNING"),
|
||||
):
|
||||
for _ in range(3):
|
||||
self.node._inject_lora_trigger_words("a cat <lora:neon:1>")
|
||||
self.assertEqual(discover.call_count, 1)
|
||||
|
||||
|
||||
class TestPromptManagerEncoding(NodeTestCase):
|
||||
node_cls = PromptManager
|
||||
|
||||
Reference in New Issue
Block a user