mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-03-21 21:22:11 -03:00
Merge pull request #616 from willmiao/codex/analyze-update-metadata-refresh-duration
fix: ignore removed civitai models during update refresh
This commit is contained in:
@@ -27,6 +27,7 @@ from ...services.service_registry import ServiceRegistry
|
||||
from ...services.settings_manager import get_settings_manager
|
||||
from ...services.websocket_manager import ws_manager
|
||||
from ...services.downloader import get_downloader
|
||||
from ...services.errors import ResourceNotFoundError
|
||||
from ...utils.constants import (
|
||||
CIVITAI_USER_MODEL_TYPES,
|
||||
DEFAULT_NODE_COLOR,
|
||||
@@ -618,7 +619,10 @@ class ModelLibraryHandler:
|
||||
if not metadata_provider:
|
||||
return web.json_response({"success": False, "error": "Metadata provider not available"}, status=503)
|
||||
|
||||
response = await metadata_provider.get_model_versions(model_id)
|
||||
try:
|
||||
response = await metadata_provider.get_model_versions(model_id)
|
||||
except ResourceNotFoundError:
|
||||
return web.json_response({"success": False, "error": "Model not found"}, status=404)
|
||||
if not response or not response.get("modelVersions"):
|
||||
return web.json_response({"success": False, "error": "Model not found"}, status=404)
|
||||
|
||||
|
||||
@@ -29,7 +29,7 @@ from ...services.use_cases import (
|
||||
)
|
||||
from ...services.websocket_manager import WebSocketManager
|
||||
from ...services.websocket_progress_callback import WebSocketProgressCallback
|
||||
from ...services.errors import RateLimitError
|
||||
from ...services.errors import RateLimitError, ResourceNotFoundError
|
||||
from ...utils.file_utils import calculate_sha256
|
||||
from ...utils.metadata_manager import MetadataManager
|
||||
|
||||
@@ -863,7 +863,10 @@ class ModelCivitaiHandler:
|
||||
try:
|
||||
model_id = request.match_info["model_id"]
|
||||
metadata_provider = await self._metadata_provider_factory()
|
||||
response = await metadata_provider.get_model_versions(model_id)
|
||||
try:
|
||||
response = await metadata_provider.get_model_versions(model_id)
|
||||
except ResourceNotFoundError:
|
||||
return web.Response(status=404, text="Model not found")
|
||||
if not response or not response.get("modelVersions"):
|
||||
return web.Response(status=404, text="Model not found")
|
||||
|
||||
|
||||
@@ -2,10 +2,10 @@ import asyncio
|
||||
import copy
|
||||
import logging
|
||||
import os
|
||||
from typing import Optional, Dict, Tuple, List, Sequence
|
||||
from typing import Any, Optional, Dict, Tuple, List, Sequence
|
||||
from .model_metadata_provider import CivitaiModelMetadataProvider, ModelMetadataProviderManager
|
||||
from .downloader import get_downloader
|
||||
from .errors import RateLimitError
|
||||
from .errors import RateLimitError, ResourceNotFoundError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -160,7 +160,29 @@ class CivitaiClient:
|
||||
logger.error(f"Download Error: {str(e)}")
|
||||
return False
|
||||
|
||||
async def get_model_versions(self, model_id: str) -> List[Dict]:
|
||||
@staticmethod
|
||||
def _extract_error_message(payload: Any) -> str:
|
||||
"""Return a human-readable error message from an API payload."""
|
||||
|
||||
def _from_value(value: Any) -> str:
|
||||
if isinstance(value, str):
|
||||
return value
|
||||
if isinstance(value, dict):
|
||||
for key in ("message", "error", "detail", "details"):
|
||||
if key in value:
|
||||
candidate = _from_value(value[key])
|
||||
if candidate:
|
||||
return candidate
|
||||
if isinstance(value, list):
|
||||
for item in value:
|
||||
candidate = _from_value(item)
|
||||
if candidate:
|
||||
return candidate
|
||||
return ""
|
||||
|
||||
return _from_value(payload)
|
||||
|
||||
async def get_model_versions(self, model_id: str) -> Optional[Dict]:
|
||||
"""Get all versions of a model with local availability info"""
|
||||
try:
|
||||
success, result = await self._make_request(
|
||||
@@ -175,12 +197,20 @@ class CivitaiClient:
|
||||
'type': result.get('type', ''),
|
||||
'name': result.get('name', '')
|
||||
}
|
||||
message = self._extract_error_message(result)
|
||||
if message and 'not found' in message.lower():
|
||||
raise ResourceNotFoundError(f"Resource not found for model {model_id}")
|
||||
if message:
|
||||
raise RuntimeError(message)
|
||||
return None
|
||||
except RateLimitError:
|
||||
raise
|
||||
except ResourceNotFoundError as exc:
|
||||
logger.info("Model %s is no longer available on Civitai: %s", model_id, exc)
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error(f"Error fetching model versions: {e}")
|
||||
return None
|
||||
logger.error("Error fetching model versions: %s", e, exc_info=True)
|
||||
raise
|
||||
|
||||
async def get_model_versions_bulk(
|
||||
self, model_ids: Sequence[int]
|
||||
|
||||
@@ -19,3 +19,9 @@ class RateLimitError(RuntimeError):
|
||||
self.retry_after = retry_after
|
||||
self.provider = provider
|
||||
|
||||
|
||||
class ResourceNotFoundError(RuntimeError):
|
||||
"""Raised when a remote resource is permanently missing."""
|
||||
|
||||
pass
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ import time
|
||||
from dataclasses import dataclass, replace
|
||||
from typing import Dict, Iterable, List, Mapping, Optional, Sequence
|
||||
|
||||
from .errors import RateLimitError
|
||||
from .errors import RateLimitError, ResourceNotFoundError
|
||||
from .settings_manager import get_settings_manager
|
||||
from ..utils.civitai_utils import rewrite_preview_url
|
||||
from ..utils.preview_selection import select_preview_media
|
||||
@@ -459,14 +459,21 @@ class ModelUpdateService:
|
||||
# release lock during network request
|
||||
fetched_versions: List[ModelVersionRecord] | None = None
|
||||
refresh_succeeded = False
|
||||
fallback_attempted = False
|
||||
fallback_error_message: Optional[str] = None
|
||||
mark_model_as_ignored = False
|
||||
response: Optional[Mapping] = None
|
||||
if metadata_provider and should_fetch:
|
||||
response = prefetched_response
|
||||
if response is None:
|
||||
fallback_attempted = True
|
||||
try:
|
||||
response = await metadata_provider.get_model_versions(model_id)
|
||||
except RateLimitError:
|
||||
raise
|
||||
except ResourceNotFoundError as exc:
|
||||
fallback_error_message = str(exc) or "resource not found"
|
||||
mark_model_as_ignored = True
|
||||
except Exception as exc: # pragma: no cover - defensive log
|
||||
logger.error(
|
||||
"Failed to fetch versions for model %s (%s): %s",
|
||||
@@ -475,11 +482,39 @@ class ModelUpdateService:
|
||||
exc,
|
||||
exc_info=True,
|
||||
)
|
||||
fallback_error_message = str(exc)
|
||||
if response is not None:
|
||||
extracted = self._extract_versions(response)
|
||||
if extracted is not None:
|
||||
fetched_versions = extracted
|
||||
refresh_succeeded = True
|
||||
elif fallback_attempted and fallback_error_message is None:
|
||||
fallback_error_message = "no versions returned"
|
||||
elif fallback_attempted and fallback_error_message is None:
|
||||
fallback_error_message = "no response"
|
||||
|
||||
if fallback_attempted:
|
||||
if refresh_succeeded and isinstance(fetched_versions, list):
|
||||
logger.info(
|
||||
"Fetched metadata via single lookup for model %s (%s); received %d versions",
|
||||
model_id,
|
||||
model_type,
|
||||
len(fetched_versions),
|
||||
)
|
||||
elif mark_model_as_ignored:
|
||||
logger.info(
|
||||
"Single lookup for model %s (%s) reported missing remote resource: %s",
|
||||
model_id,
|
||||
model_type,
|
||||
fallback_error_message or "resource not found",
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
"Single lookup for model %s (%s) failed: %s",
|
||||
model_id,
|
||||
model_type,
|
||||
fallback_error_message or "unknown error",
|
||||
)
|
||||
|
||||
async with self._lock:
|
||||
existing = self._get_record(model_type, model_id)
|
||||
@@ -491,6 +526,23 @@ class ModelUpdateService:
|
||||
self._upsert_record(record)
|
||||
return record
|
||||
|
||||
if mark_model_as_ignored:
|
||||
record = self._merge_with_local_versions(
|
||||
existing,
|
||||
normalized_local,
|
||||
model_type=model_type,
|
||||
model_id=model_id,
|
||||
last_checked_at=now,
|
||||
)
|
||||
record = replace(record, should_ignore_model=True)
|
||||
self._upsert_record(record)
|
||||
logger.info(
|
||||
"Marked model %s (%s) as ignored after remote resource was not found",
|
||||
model_id,
|
||||
model_type,
|
||||
)
|
||||
return record
|
||||
|
||||
if refresh_succeeded and isinstance(fetched_versions, list):
|
||||
record = self._build_record_from_remote(
|
||||
model_type,
|
||||
|
||||
@@ -5,7 +5,7 @@ import pytest
|
||||
|
||||
from py.services import civitai_client as civitai_client_module
|
||||
from py.services.civitai_client import CivitaiClient
|
||||
from py.services.errors import RateLimitError
|
||||
from py.services.errors import RateLimitError, ResourceNotFoundError
|
||||
from py.services.model_metadata_provider import ModelMetadataProviderManager
|
||||
|
||||
|
||||
@@ -162,6 +162,42 @@ async def test_get_model_versions_success(monkeypatch, downloader):
|
||||
assert result == {"modelVersions": [{"id": 1}], "type": "LORA", "name": "Model"}
|
||||
|
||||
|
||||
async def test_get_model_versions_raises_on_not_found(monkeypatch, downloader):
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
return False, {"message": "Resource not found"}
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
client = await CivitaiClient.get_instance()
|
||||
|
||||
with pytest.raises(ResourceNotFoundError):
|
||||
await client.get_model_versions("missing")
|
||||
|
||||
|
||||
async def test_get_model_versions_raises_on_nested_not_found(monkeypatch, downloader):
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
return False, {"error": {"message": "Resource not found"}}
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
client = await CivitaiClient.get_instance()
|
||||
|
||||
with pytest.raises(ResourceNotFoundError):
|
||||
await client.get_model_versions("missing")
|
||||
|
||||
|
||||
async def test_get_model_versions_raises_on_other_errors(monkeypatch, downloader):
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
return False, {"error": {"message": "Server error"}}
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
client = await CivitaiClient.get_instance()
|
||||
|
||||
with pytest.raises(RuntimeError):
|
||||
await client.get_model_versions("oops")
|
||||
|
||||
|
||||
async def test_get_model_versions_bulk_success(monkeypatch, downloader):
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
assert url.endswith("/models")
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import logging
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from py.services.errors import ResourceNotFoundError
|
||||
from py.services.model_update_service import (
|
||||
ModelUpdateRecord,
|
||||
ModelUpdateService,
|
||||
@@ -35,6 +37,20 @@ class DummyProvider:
|
||||
return {model_id: self.response for model_id in model_ids}
|
||||
|
||||
|
||||
class NotFoundProvider:
|
||||
def __init__(self):
|
||||
self.calls = 0
|
||||
self.bulk_calls: list[list[int]] = []
|
||||
|
||||
async def get_model_versions(self, model_id):
|
||||
self.calls += 1
|
||||
raise ResourceNotFoundError("Resource not found")
|
||||
|
||||
async def get_model_versions_bulk(self, model_ids):
|
||||
self.bulk_calls.append(list(model_ids))
|
||||
return {}
|
||||
|
||||
|
||||
def make_version(version_id, *, in_library, should_ignore=False):
|
||||
return ModelVersionRecord(
|
||||
version_id=version_id,
|
||||
@@ -155,6 +171,43 @@ async def test_refresh_respects_ignore_flag(tmp_path):
|
||||
assert record.should_ignore_model is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_refresh_marks_model_ignored_when_remote_missing(tmp_path):
|
||||
db_path = tmp_path / "updates.sqlite"
|
||||
service = ModelUpdateService(str(db_path), ttl_seconds=3600)
|
||||
raw_data = [{"civitai": {"modelId": 5, "id": 51}}]
|
||||
scanner = DummyScanner(raw_data)
|
||||
provider = NotFoundProvider()
|
||||
|
||||
await service.refresh_for_model_type("lora", scanner, provider)
|
||||
record = await service.get_record("lora", 5)
|
||||
|
||||
assert provider.bulk_calls == [[5]]
|
||||
assert provider.calls == 1
|
||||
assert record is not None
|
||||
assert record.should_ignore_model is True
|
||||
assert record.in_library_version_ids == [51]
|
||||
assert record.last_checked_at is not None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_refresh_logs_info_for_missing_remote(tmp_path, caplog):
|
||||
db_path = tmp_path / "updates.sqlite"
|
||||
service = ModelUpdateService(str(db_path), ttl_seconds=3600)
|
||||
raw_data = [{"civitai": {"modelId": 6, "id": 61}}]
|
||||
scanner = DummyScanner(raw_data)
|
||||
provider = NotFoundProvider()
|
||||
|
||||
with caplog.at_level(logging.INFO, logger="py.services.model_update_service"):
|
||||
await service.refresh_for_model_type("lora", scanner, provider)
|
||||
|
||||
relevant = [
|
||||
record for record in caplog.records if "Single lookup for model" in record.message
|
||||
]
|
||||
assert relevant, "expected single lookup log entry"
|
||||
assert all(record.levelno == logging.INFO for record in relevant)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_refresh_falls_back_when_bulk_not_supported(tmp_path):
|
||||
db_path = tmp_path / "updates.sqlite"
|
||||
|
||||
Reference in New Issue
Block a user