mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-08 18:42:12 -03:00
fix(metadata): fail loudly when the sidecar write cannot land
update_model_metadata ignored the save_metadata result, so a fetch on a drive that went offline mid-run was counted as success: the cache got the fresh CivitAI payload while the durable sidecar never landed, and every later fetch skipped the model as already fetched. The save result now raises MetadataPersistError. In the bulk fetch path this fails the item (reported in the summary, cache untouched, model stays eligible for the next run); relink surfaces an honest 500. Also corrects the stale known-limitation note in the scoped-scan plan: a startup-offline root has been listed and re-admitted since Wave 6.
This commit is contained in:
@@ -364,8 +364,9 @@ differs per mode and that matters:
|
|||||||
unioned rather than replaced whenever the scan did not verify every root); the next full refresh
|
unioned rather than replaced whenever the scan did not verify every root); the next full refresh
|
||||||
drops them.
|
drops them.
|
||||||
- `root_details[].reachable` is a live `os.path.exists()` per root, so a root that disappears
|
- `root_details[].reachable` is a live `os.path.exists()` per root, so a root that disappears
|
||||||
mid-session shows as offline; a root that was already gone at startup is filtered out by
|
mid-session shows as offline; a root that was already gone at startup is still listed thanks
|
||||||
`Config` and therefore absent from the list.
|
to Wave 6 (`available: false`, cached count), and `/roots` re-admits it once the directory
|
||||||
|
is back.
|
||||||
|
|
||||||
## Verification checklist
|
## Verification checklist
|
||||||
|
|
||||||
|
|||||||
@@ -45,6 +45,12 @@ class ResourceNotFoundError(RuntimeError):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class MetadataPersistError(RuntimeError):
|
||||||
|
"""Raised when the metadata sidecar cannot be written to disk."""
|
||||||
|
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
class LLMNotConfiguredError(RuntimeError):
|
class LLMNotConfiguredError(RuntimeError):
|
||||||
"""Raised when an LLM-dependent operation is attempted but no provider is configured."""
|
"""Raised when an LLM-dependent operation is attempted but no provider is configured."""
|
||||||
|
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ from ..utils.model_utils import determine_base_model
|
|||||||
from ..utils.models import autov3_from_civitai_files
|
from ..utils.models import autov3_from_civitai_files
|
||||||
from ..utils.sidecar_paths import get_metadata_path
|
from ..utils.sidecar_paths import get_metadata_path
|
||||||
from .connectivity_guard import OFFLINE_FRIENDLY_MESSAGE, is_expected_offline_error
|
from .connectivity_guard import OFFLINE_FRIENDLY_MESSAGE, is_expected_offline_error
|
||||||
from .errors import RateLimitError
|
from .errors import MetadataPersistError, RateLimitError
|
||||||
from .model_metadata_provider import _LOCAL_PROVIDER_LABELS
|
from .model_metadata_provider import _LOCAL_PROVIDER_LABELS
|
||||||
from .model_sources import get_source_platform, has_external_source
|
from .model_sources import get_source_platform, has_external_source
|
||||||
|
|
||||||
@@ -231,7 +231,16 @@ class MetadataSyncService:
|
|||||||
metadata_path, local_metadata, civitai_metadata.get("images", [])
|
metadata_path, local_metadata, civitai_metadata.get("images", [])
|
||||||
)
|
)
|
||||||
|
|
||||||
await self._metadata_manager.save_metadata(metadata_path, local_metadata)
|
saved = await self._metadata_manager.save_metadata(metadata_path, local_metadata)
|
||||||
|
if not saved:
|
||||||
|
# A swallowed write failure would update the cache while the
|
||||||
|
# durable sidecar never lands (e.g. the drive went offline), and
|
||||||
|
# the model would be skipped by every later fetch as "already
|
||||||
|
# fetched". Fail loudly instead so the caller reports the item
|
||||||
|
# and it stays eligible for the next run.
|
||||||
|
raise MetadataPersistError(
|
||||||
|
f"Failed to write metadata sidecar: {metadata_path}"
|
||||||
|
)
|
||||||
return local_metadata
|
return local_metadata
|
||||||
|
|
||||||
async def fetch_and_update_model(
|
async def fetch_and_update_model(
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ from unittest.mock import AsyncMock
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from py.services.connectivity_guard import OFFLINE_COOLDOWN_ERROR, OFFLINE_FRIENDLY_MESSAGE
|
from py.services.connectivity_guard import OFFLINE_COOLDOWN_ERROR, OFFLINE_FRIENDLY_MESSAGE
|
||||||
from py.services.errors import RateLimitError
|
from py.services.errors import MetadataPersistError, RateLimitError
|
||||||
from py.services.metadata_sync_service import MetadataSyncService, _merge_ordered_unique
|
from py.services.metadata_sync_service import MetadataSyncService, _merge_ordered_unique
|
||||||
|
|
||||||
|
|
||||||
@@ -285,6 +285,56 @@ async def test_fetch_and_update_model_success_updates_cache(tmp_path):
|
|||||||
update_cache.assert_awaited_once()
|
update_cache.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_update_model_metadata_raises_when_sidecar_write_fails():
|
||||||
|
"""A failed sidecar write must surface instead of being swallowed."""
|
||||||
|
helpers = build_service()
|
||||||
|
helpers.metadata_manager.save_metadata.return_value = False
|
||||||
|
|
||||||
|
with pytest.raises(MetadataPersistError):
|
||||||
|
await helpers.service.update_model_metadata(
|
||||||
|
"path/to/model.metadata.json",
|
||||||
|
{"civitai": {}, "model_name": "Local"},
|
||||||
|
{"source": "api", "model": {"name": "Remote"}, "images": []},
|
||||||
|
helpers.default_provider,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_fetch_and_update_model_reports_sidecar_write_failure(tmp_path):
|
||||||
|
"""Fetch fails the item (and leaves the cache untouched) when the sidecar
|
||||||
|
cannot be written, so the model stays eligible for the next run."""
|
||||||
|
helpers = build_service()
|
||||||
|
helpers.metadata_manager.save_metadata.return_value = False
|
||||||
|
|
||||||
|
civitai_payload = {
|
||||||
|
"source": "api",
|
||||||
|
"model": {"name": "Remote", "description": "", "tags": ["tag"]},
|
||||||
|
"images": [],
|
||||||
|
"baseModel": "sdxl",
|
||||||
|
}
|
||||||
|
helpers.default_provider.get_model_by_hash.return_value = (civitai_payload, None)
|
||||||
|
|
||||||
|
model_path = tmp_path / "model.safetensors"
|
||||||
|
model_data: Dict[str, Any] = {
|
||||||
|
"model_name": "Local",
|
||||||
|
"folder": "root",
|
||||||
|
"file_path": str(model_path),
|
||||||
|
}
|
||||||
|
update_cache = AsyncMock(return_value=True)
|
||||||
|
|
||||||
|
ok, error = await helpers.service.fetch_and_update_model(
|
||||||
|
sha256="abc",
|
||||||
|
file_path=str(model_path),
|
||||||
|
model_data=model_data,
|
||||||
|
update_cache_func=update_cache,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ok is False
|
||||||
|
assert "sidecar" in (error or "")
|
||||||
|
update_cache.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_fetch_and_update_model_keeps_deleted_flag_false_for_archive_source(tmp_path):
|
async def test_fetch_and_update_model_keeps_deleted_flag_false_for_archive_source(tmp_path):
|
||||||
helpers = build_service()
|
helpers = build_service()
|
||||||
|
|||||||
Reference in New Issue
Block a user