mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-03-25 07:05:43 -03:00
Revert "feat: add automatic retry for failed example image downloads"
This reverts commit cb460fcdb0.
This commit is contained in:
Will Miao
File diff suppressed because it is too large
Load Diff
@@ -1,287 +0,0 @@
|
|||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import time
|
|
||||||
from typing import Any, Dict
|
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from py.utils import example_images_download_manager as download_module
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
|
||||||
def restore_settings():
|
|
||||||
from py.services.settings_manager import get_settings_manager
|
|
||||||
|
|
||||||
manager = get_settings_manager()
|
|
||||||
original = manager.settings.copy()
|
|
||||||
try:
|
|
||||||
yield
|
|
||||||
finally:
|
|
||||||
manager.settings.clear()
|
|
||||||
manager.settings.update(original)
|
|
||||||
|
|
||||||
|
|
||||||
class RecordingWebSocketManager:
|
|
||||||
def __init__(self) -> None:
|
|
||||||
self.payloads: list[Dict[str, Any]] = []
|
|
||||||
|
|
||||||
async def broadcast(self, payload: Dict[str, Any]) -> None:
|
|
||||||
self.payloads.append(payload)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_process_model_with_old_failure_and_empty_folder_retries():
|
|
||||||
"""Test that models with old failures and empty folders are removed from failed list for retry."""
|
|
||||||
from py.services.settings_manager import get_settings_manager
|
|
||||||
from py.services.downloader import get_downloader
|
|
||||||
|
|
||||||
settings_manager = get_settings_manager()
|
|
||||||
settings_manager.settings["libraries"] = {"default": {}}
|
|
||||||
settings_manager.settings["active_library"] = "default"
|
|
||||||
|
|
||||||
ws_manager = RecordingWebSocketManager()
|
|
||||||
manager = download_module.DownloadManager(ws_manager=ws_manager)
|
|
||||||
|
|
||||||
# Create a model hash
|
|
||||||
test_hash = "test_hash_12345678"
|
|
||||||
test_name = "Test Model"
|
|
||||||
|
|
||||||
# Mark as failed with timestamp 25 hours ago
|
|
||||||
old_timestamp = time.time() - (25 * 60 * 60)
|
|
||||||
manager._progress["failed_models"].add(test_hash)
|
|
||||||
manager._progress["failed_model_timestamps"][test_hash] = old_timestamp
|
|
||||||
|
|
||||||
# Verify initial state - model is in failed list
|
|
||||||
assert test_hash in manager._progress["failed_models"]
|
|
||||||
assert test_hash in manager._progress["failed_model_timestamps"]
|
|
||||||
initial_timestamp = manager._progress["failed_model_timestamps"][test_hash]
|
|
||||||
|
|
||||||
# Mock dependencies to make model successfully download
|
|
||||||
mock_scanner = MagicMock()
|
|
||||||
mock_model = {
|
|
||||||
"sha256": test_hash,
|
|
||||||
"model_name": test_name,
|
|
||||||
"file_path": "/fake/path/model.safetensors",
|
|
||||||
"file_name": "model.safetensors",
|
|
||||||
"civitai": {"images": [{"url": "http://example.com/image.jpg"}]},
|
|
||||||
}
|
|
||||||
|
|
||||||
mock_downloader = await get_downloader()
|
|
||||||
|
|
||||||
# Mock path resolver to return directory with existing files
|
|
||||||
# This will make the code skip retry but verify the logic works
|
|
||||||
with (
|
|
||||||
patch.object(
|
|
||||||
download_module.ExampleImagePathResolver,
|
|
||||||
"get_model_folder",
|
|
||||||
return_value="/fake/dir/with/files",
|
|
||||||
),
|
|
||||||
patch.object(
|
|
||||||
download_module,
|
|
||||||
"_model_directory_has_files",
|
|
||||||
return_value=True, # Files exist
|
|
||||||
),
|
|
||||||
):
|
|
||||||
result = await manager._process_model(
|
|
||||||
"lora",
|
|
||||||
mock_model,
|
|
||||||
mock_scanner,
|
|
||||||
"/fake/output",
|
|
||||||
False,
|
|
||||||
mock_downloader,
|
|
||||||
"default",
|
|
||||||
)
|
|
||||||
|
|
||||||
# When files exist, model should remain in failed list (not retried)
|
|
||||||
assert test_hash in manager._progress["failed_models"]
|
|
||||||
assert test_hash in manager._progress["failed_model_timestamps"]
|
|
||||||
# Result should be False because no remote download happened (skipped due to existing files)
|
|
||||||
assert result is False
|
|
||||||
|
|
||||||
# Now test the actual retry path by mocking empty directory
|
|
||||||
manager._progress["processed_models"].clear()
|
|
||||||
manager._progress["failed_models"].clear()
|
|
||||||
manager._progress["failed_model_timestamps"].clear()
|
|
||||||
|
|
||||||
# Re-add to failed with old timestamp
|
|
||||||
manager._progress["failed_models"].add(test_hash)
|
|
||||||
manager._progress["failed_model_timestamps"][test_hash] = old_timestamp
|
|
||||||
|
|
||||||
with (
|
|
||||||
patch.object(
|
|
||||||
download_module.ExampleImagePathResolver,
|
|
||||||
"get_model_folder",
|
|
||||||
return_value="/fake/empty/dir",
|
|
||||||
),
|
|
||||||
patch.object(
|
|
||||||
download_module,
|
|
||||||
"_model_directory_has_files",
|
|
||||||
return_value=False, # No files
|
|
||||||
),
|
|
||||||
patch.object(
|
|
||||||
download_module.ExampleImagesProcessor,
|
|
||||||
"process_local_examples",
|
|
||||||
new_callable=AsyncMock,
|
|
||||||
return_value=False,
|
|
||||||
),
|
|
||||||
# Note: We don't mock download_model_images_with_tracking here
|
|
||||||
# because it's complex. The key thing is that the model is
|
|
||||||
# removed from failed list so it can be retried.
|
|
||||||
):
|
|
||||||
# Just check that the model is removed from failed list before processing
|
|
||||||
# This proves the retry logic is triggered
|
|
||||||
result = await manager._process_model(
|
|
||||||
"lora",
|
|
||||||
mock_model,
|
|
||||||
mock_scanner,
|
|
||||||
"/fake/output",
|
|
||||||
False,
|
|
||||||
mock_downloader,
|
|
||||||
"default",
|
|
||||||
)
|
|
||||||
|
|
||||||
# The model should have been removed from failed_models for retry
|
|
||||||
# (even if it gets re-added later due to download failure)
|
|
||||||
assert (
|
|
||||||
result is False
|
|
||||||
or test_hash not in manager._progress["failed_models"]
|
|
||||||
or test_hash in manager._progress["processed_models"]
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_process_model_with_old_failure_and_existing_files_skips():
|
|
||||||
"""Test that models with old failures but existing files are not retried."""
|
|
||||||
from py.services.settings_manager import get_settings_manager
|
|
||||||
from py.services.downloader import get_downloader
|
|
||||||
|
|
||||||
settings_manager = get_settings_manager()
|
|
||||||
settings_manager.settings["libraries"] = {"default": {}}
|
|
||||||
settings_manager.settings["active_library"] = "default"
|
|
||||||
|
|
||||||
ws_manager = RecordingWebSocketManager()
|
|
||||||
manager = download_module.DownloadManager(ws_manager=ws_manager)
|
|
||||||
|
|
||||||
# Create a model hash
|
|
||||||
test_hash = "test_hash_12345678"
|
|
||||||
test_name = "Test Model"
|
|
||||||
|
|
||||||
# Mark as failed with timestamp 25 hours ago
|
|
||||||
old_timestamp = time.time() - (25 * 60 * 60)
|
|
||||||
manager._progress["failed_models"].add(test_hash)
|
|
||||||
manager._progress["failed_model_timestamps"][test_hash] = old_timestamp
|
|
||||||
|
|
||||||
mock_scanner = MagicMock()
|
|
||||||
mock_model = {
|
|
||||||
"sha256": test_hash,
|
|
||||||
"model_name": test_name,
|
|
||||||
"file_path": "/fake/path/model.safetensors",
|
|
||||||
"file_name": "model.safetensors",
|
|
||||||
}
|
|
||||||
|
|
||||||
# Mock path resolver to return directory with files
|
|
||||||
with (
|
|
||||||
patch.object(
|
|
||||||
download_module.ExampleImagePathResolver,
|
|
||||||
"get_model_folder",
|
|
||||||
return_value="/fake/dir/with/files",
|
|
||||||
),
|
|
||||||
patch.object(
|
|
||||||
download_module,
|
|
||||||
"_model_directory_has_files",
|
|
||||||
return_value=True,
|
|
||||||
),
|
|
||||||
):
|
|
||||||
result = await manager._process_model(
|
|
||||||
"lora",
|
|
||||||
mock_model,
|
|
||||||
mock_scanner,
|
|
||||||
"/fake/output",
|
|
||||||
False,
|
|
||||||
await get_downloader(),
|
|
||||||
"default",
|
|
||||||
)
|
|
||||||
|
|
||||||
# Verify model is still in failed list (not retried because files exist)
|
|
||||||
assert test_hash in manager._progress["failed_models"]
|
|
||||||
assert test_hash in manager._progress["failed_model_timestamps"]
|
|
||||||
assert result is False # No remote download happened
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_process_model_with_recent_failure_skips():
|
|
||||||
"""Test that models with recent failures are not retried."""
|
|
||||||
from py.services.settings_manager import get_settings_manager
|
|
||||||
from py.services.downloader import get_downloader
|
|
||||||
|
|
||||||
settings_manager = get_settings_manager()
|
|
||||||
settings_manager.settings["libraries"] = {"default": {}}
|
|
||||||
settings_manager.settings["active_library"] = "default"
|
|
||||||
|
|
||||||
ws_manager = RecordingWebSocketManager()
|
|
||||||
manager = download_module.DownloadManager(ws_manager=ws_manager)
|
|
||||||
|
|
||||||
# Create a model hash
|
|
||||||
test_hash = "test_hash_12345678"
|
|
||||||
test_name = "Test Model"
|
|
||||||
|
|
||||||
# Mark as failed with timestamp 2 hours ago (recent)
|
|
||||||
recent_timestamp = time.time() - (2 * 60 * 60)
|
|
||||||
manager._progress["failed_models"].add(test_hash)
|
|
||||||
manager._progress["failed_model_timestamps"][test_hash] = recent_timestamp
|
|
||||||
|
|
||||||
mock_scanner = MagicMock()
|
|
||||||
mock_model = {
|
|
||||||
"sha256": test_hash,
|
|
||||||
"model_name": test_name,
|
|
||||||
"file_path": "/fake/path/model.safetensors",
|
|
||||||
"file_name": "model.safetensors",
|
|
||||||
}
|
|
||||||
|
|
||||||
# Mock path resolver to return empty directory
|
|
||||||
with (
|
|
||||||
patch.object(
|
|
||||||
download_module.ExampleImagePathResolver,
|
|
||||||
"get_model_folder",
|
|
||||||
return_value="/fake/empty/dir",
|
|
||||||
),
|
|
||||||
patch.object(
|
|
||||||
download_module,
|
|
||||||
"_model_directory_has_files",
|
|
||||||
return_value=False,
|
|
||||||
),
|
|
||||||
):
|
|
||||||
result = await manager._process_model(
|
|
||||||
"lora",
|
|
||||||
mock_model,
|
|
||||||
mock_scanner,
|
|
||||||
"/fake/output",
|
|
||||||
False,
|
|
||||||
await get_downloader(),
|
|
||||||
"default",
|
|
||||||
)
|
|
||||||
|
|
||||||
# Verify model is still in failed list (not retried because too recent)
|
|
||||||
assert test_hash in manager._progress["failed_models"]
|
|
||||||
assert test_hash in manager._progress["failed_model_timestamps"]
|
|
||||||
assert result is False # No remote download happened
|
|
||||||
|
|
||||||
|
|
||||||
def test_progress_includes_failed_timestamps():
|
|
||||||
"""Test that _DownloadProgress includes failed_model_timestamps."""
|
|
||||||
progress = download_module._DownloadProgress()
|
|
||||||
progress.reset()
|
|
||||||
|
|
||||||
assert "failed_model_timestamps" in progress
|
|
||||||
assert isinstance(progress["failed_model_timestamps"], dict)
|
|
||||||
|
|
||||||
|
|
||||||
def test_progress_snapshot_excludes_failed_timestamps():
|
|
||||||
"""Test that snapshot() excludes failed_model_timestamps."""
|
|
||||||
progress = download_module._DownloadProgress()
|
|
||||||
progress.reset()
|
|
||||||
|
|
||||||
snapshot = progress.snapshot()
|
|
||||||
|
|
||||||
assert "failed_model_timestamps" not in snapshot
|
|
||||||
Reference in New Issue
Block a user