mirror of
https://github.com/Comfy-Org/ComfyUI.git
synced 2026-09-21 05:27:57 -05:00
f42b24efbe
* feat(assets): structured event log lines for the assets system * test(assets): AST check that tagged event lines stay inside the closed vocabulary * feat(assets): structured event logging for the seeder lifecycle * feat(assets): structured event logging for scanner and ingest failure paths * feat(assets): structured event logging for API request failures * fix(seeder): finalize checkpoint no longer parks completed scans * fix(scanner): enrich_failed counts exceptions, not benign races * fix(seeder): idle reset survives emit/assert failures * refactor(assets): private emit-once bookkeeping helper; clean Progress DTO * test(event-log): call-site registry as frozenset; drop dead pending machinery * feat(event-log): scanner.stat_failed with emit-once discipline; drop dead vocabulary * refactor(assets): drop API request failure events * refactor(assets): drop ingest failure events * refactor(assets): narrow event vocabulary to scan pipeline * test(assets): per-call-site event tests collapse to mechanism-once * refactor(assets): event log lines as logfmt, matching the log's own idiom * test(assets): adapt hash-failure log assertion to privacy-safe scan-error logging The rebase onto master picked up the privacy-safe _log_scan_error from PR 16096, which no longer includes the file path in the log message. Assert on the generic 'Asset scan error' message and that the path does not leak, instead of asserting the path is present. * fix(assets): enforce event vocabulary and scan counters --------- Co-authored-by: Simon Pinfold <synap5e@users.noreply.github.com> Co-authored-by: guill <jacob.e.segal@gmail.com>
437 lines
14 KiB
Python
437 lines
14 KiB
Python
import logging
|
|
import re
|
|
from contextlib import nullcontext
|
|
from pathlib import Path
|
|
from types import SimpleNamespace
|
|
from unittest.mock import Mock
|
|
|
|
import pytest
|
|
|
|
from app.assets import scanner
|
|
from app.assets.event_log import TAG
|
|
from app.assets.scanner import UnenrichedContent
|
|
from app.assets.seeder import _ScanState
|
|
|
|
|
|
EVENT_LINE_PATTERN = re.compile(
|
|
rf"^{re.escape(TAG)} (?P<event>[a-z][a-z0-9_]*(?:\.[a-z][a-z0-9_]*)*)"
|
|
r"(?P<fields>(?: [a-z_]+=[^ =]+)*)$"
|
|
)
|
|
EventFields = dict[str, bool | int | str]
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def autoclean_unit_test_assets():
|
|
yield
|
|
|
|
|
|
def parse_fields(raw: str) -> EventFields:
|
|
fields: EventFields = {}
|
|
for pair in raw.split():
|
|
name, value = pair.split("=", maxsplit=1)
|
|
if value == "true":
|
|
fields[name] = True
|
|
elif value == "false":
|
|
fields[name] = False
|
|
elif value.removeprefix("-").isdigit():
|
|
fields[name] = int(value)
|
|
else:
|
|
fields[name] = value
|
|
return fields
|
|
|
|
|
|
def tagged_events(caplog: pytest.LogCaptureFixture) -> list[tuple[str, EventFields]]:
|
|
events: list[tuple[str, EventFields]] = []
|
|
for record in caplog.records:
|
|
match = EVENT_LINE_PATTERN.match(record.getMessage())
|
|
if match is not None:
|
|
events.append((match.group("event"), parse_fields(match.group("fields"))))
|
|
return events
|
|
|
|
|
|
def events_named(
|
|
caplog: pytest.LogCaptureFixture, event_name: str
|
|
) -> list[EventFields]:
|
|
return [fields for event, fields in tagged_events(caplog) if event == event_name]
|
|
|
|
|
|
def tagged_lines(caplog: pytest.LogCaptureFixture) -> list[str]:
|
|
return [record.getMessage() for record in caplog.records if record.getMessage().startswith(TAG)]
|
|
|
|
|
|
def hash_session(path: Path) -> Mock:
|
|
stat_result = path.stat()
|
|
content = SimpleNamespace(hash=None, mtime_ns=stat_result.st_mtime_ns)
|
|
record = SimpleNamespace(system_metadata=None, mime_type=None)
|
|
session = Mock()
|
|
session.get.side_effect = lambda _model, row_id: content if row_id == "content" else record
|
|
return session
|
|
|
|
|
|
def run_hash_failure(session: Mock, path: Path, progress: _ScanState) -> bool:
|
|
return scanner.enrich_asset(
|
|
session,
|
|
file_path=str(path),
|
|
content_id="content",
|
|
record_id="record",
|
|
extract_metadata=False,
|
|
compute_hash=True,
|
|
progress=progress,
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("operation", "event_name", "expected_fields", "expected_result"),
|
|
[
|
|
pytest.param(
|
|
lambda: scanner.sync_root_safely("models"),
|
|
"scanner.fast_scan_failed",
|
|
{"error_type": "FileNotFoundError", "root": "models"},
|
|
set(),
|
|
id="fast-scan",
|
|
),
|
|
],
|
|
)
|
|
def test_scanner_safe_failures_emit_exception_type_without_path(
|
|
operation,
|
|
event_name: str,
|
|
expected_fields: EventFields,
|
|
expected_result,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
secret_path = "/private/assets/secret.safetensors"
|
|
|
|
def fail_session():
|
|
raise FileNotFoundError(secret_path)
|
|
|
|
monkeypatch.setattr(scanner, "create_session", fail_session)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
result = operation()
|
|
|
|
assert result == expected_result
|
|
assert events_named(caplog, event_name) == [expected_fields]
|
|
assert all(secret_path not in line for line in tagged_lines(caplog))
|
|
|
|
|
|
def test_permission_error_in_reference_sync_increments_scan_counter(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
secret_path = "/private/assets/unreadable.safetensors"
|
|
content = SimpleNamespace(id="content", path=secret_path)
|
|
progress = _ScanState()
|
|
|
|
def deny_stat(*_args, **_kwargs):
|
|
raise PermissionError(secret_path)
|
|
|
|
monkeypatch.setattr(scanner, "os", SimpleNamespace(stat=deny_stat, path=scanner.os.path))
|
|
monkeypatch.setattr(scanner, "live_contents_under_prefixes", lambda _session, _prefixes: [content])
|
|
|
|
scanner.sync_prefixes_with_filesystem(Mock(), ["/private/assets"], progress=progress)
|
|
|
|
assert progress.permission_denied == 1
|
|
|
|
|
|
def test_hash_failures_emit_once_per_scan_and_count_every_failure(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
path = tmp_path / "model.safetensors"
|
|
path.write_bytes(b"model")
|
|
progress = _ScanState()
|
|
|
|
def fail_hash(_path: str):
|
|
raise FileNotFoundError(str(path))
|
|
|
|
monkeypatch.setattr(scanner, "snapshot_hash", fail_hash)
|
|
session = hash_session(path)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
assert run_hash_failure(session, path, progress) is False
|
|
assert run_hash_failure(session, path, progress) is False
|
|
|
|
assert events_named(caplog, "scanner.hash_failed") == [
|
|
{"error_type": "FileNotFoundError"}
|
|
]
|
|
assert progress.hash_failed == 2
|
|
|
|
|
|
def test_hash_failure_first_occurrence_resets_with_new_scan(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
path = tmp_path / "model.safetensors"
|
|
path.write_bytes(b"model")
|
|
|
|
def fail_hash(_path: str):
|
|
raise OSError("hash unavailable")
|
|
|
|
monkeypatch.setattr(scanner, "snapshot_hash", fail_hash)
|
|
session = hash_session(path)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
run_hash_failure(session, path, _ScanState())
|
|
run_hash_failure(session, path, _ScanState())
|
|
|
|
assert events_named(caplog, "scanner.hash_failed") == [
|
|
{"error_type": "OSError"},
|
|
{"error_type": "OSError"},
|
|
]
|
|
|
|
|
|
def test_hash_failure_no_log_line_leaks_exception_path(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
# Both the tagged [assets-event] line and the privacy-safe
|
|
# "Asset scan error" line must omit the file path.
|
|
path = tmp_path / "private-model.safetensors"
|
|
path.write_bytes(b"model")
|
|
|
|
def fail_hash(_path: str):
|
|
raise FileNotFoundError(str(path))
|
|
|
|
monkeypatch.setattr(scanner, "snapshot_hash", fail_hash)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
run_hash_failure(hash_session(path), path, _ScanState())
|
|
|
|
assert events_named(caplog, "scanner.hash_failed") == [
|
|
{"error_type": "FileNotFoundError"}
|
|
]
|
|
assert any("Asset scan error" in record.getMessage() for record in caplog.records)
|
|
assert all(str(path) not in record.getMessage() for record in caplog.records)
|
|
assert all(str(path) not in line for line in tagged_lines(caplog))
|
|
|
|
|
|
def test_modified_during_hash_emits_fieldless_discard_event(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
path = tmp_path / "changing.safetensors"
|
|
path.write_bytes(b"model")
|
|
monkeypatch.setattr(scanner, "snapshot_hash", lambda _path: None)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
updated = scanner.enrich_asset(
|
|
hash_session(path),
|
|
file_path=str(path),
|
|
content_id="content",
|
|
record_id="record",
|
|
extract_metadata=False,
|
|
compute_hash=True,
|
|
progress=_ScanState(),
|
|
)
|
|
|
|
assert updated is False
|
|
assert events_named(caplog, "scanner.hash_discarded_modified") == [{}]
|
|
|
|
|
|
def test_modified_hash_event_emits_once_per_scan_and_resets(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
path = tmp_path / "changing.safetensors"
|
|
path.write_bytes(b"model")
|
|
monkeypatch.setattr(scanner, "snapshot_hash", lambda _path: None)
|
|
progress = _ScanState()
|
|
session = hash_session(path)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
assert run_hash_failure(session, path, progress) is False
|
|
assert run_hash_failure(session, path, progress) is False
|
|
assert events_named(caplog, "scanner.hash_discarded_modified") == [{}]
|
|
assert run_hash_failure(session, path, _ScanState()) is False
|
|
|
|
assert events_named(caplog, "scanner.hash_discarded_modified") == [{}, {}]
|
|
assert progress.hash_failed == 0
|
|
|
|
|
|
def test_locked_files_during_discovery_emit_stat_failed_exactly_once(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
def deny_stat(*_args, **_kwargs):
|
|
raise PermissionError("/private/assets/secret.bin")
|
|
|
|
monkeypatch.setattr(scanner, "os", SimpleNamespace(stat=deny_stat, path=scanner.os.path))
|
|
progress = _ScanState()
|
|
paths = [f"/private/assets/locked-{i}.bin" for i in range(3)]
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
specs, _tag_pool, _skipped = scanner.build_asset_specs(paths, set(), progress=progress)
|
|
|
|
assert specs == []
|
|
assert events_named(caplog, "scanner.stat_failed") == [
|
|
{"error_type": "PermissionError", "site": "discovery"}
|
|
]
|
|
assert progress.permission_denied == 3
|
|
|
|
|
|
def test_missing_files_during_discovery_emit_nothing(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
def vanish_stat(*_args, **_kwargs):
|
|
raise FileNotFoundError("/private/assets/gone.bin")
|
|
|
|
monkeypatch.setattr(scanner, "os", SimpleNamespace(stat=vanish_stat, path=scanner.os.path))
|
|
progress = _ScanState()
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
specs, _tag_pool, _skipped = scanner.build_asset_specs(
|
|
["/private/assets/gone.bin"], set(), progress=progress
|
|
)
|
|
|
|
assert specs == []
|
|
assert events_named(caplog, "scanner.stat_failed") == []
|
|
|
|
|
|
def test_discovery_stat_failure_emits_nothing_when_progress_is_none(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
def deny_stat(*_args, **_kwargs):
|
|
raise PermissionError("/private/assets/secret.bin")
|
|
|
|
monkeypatch.setattr(scanner, "os", SimpleNamespace(stat=deny_stat, path=scanner.os.path))
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
scanner.build_asset_specs(["/private/assets/locked.bin"], set(), progress=None)
|
|
|
|
assert events_named(caplog, "scanner.stat_failed") == []
|
|
|
|
|
|
def test_locked_files_during_enrichment_emit_stat_failed_exactly_once(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
def deny_stat(*_args, **_kwargs):
|
|
raise PermissionError("/private/assets/secret.bin")
|
|
|
|
monkeypatch.setattr(scanner, "os", SimpleNamespace(stat=deny_stat, path=scanner.os.path))
|
|
progress = _ScanState()
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
first = scanner.enrich_asset(
|
|
Mock(),
|
|
file_path="/private/assets/locked-1.bin",
|
|
content_id="content-1",
|
|
record_id="record-1",
|
|
extract_metadata=False,
|
|
compute_hash=False,
|
|
progress=progress,
|
|
)
|
|
second = scanner.enrich_asset(
|
|
Mock(),
|
|
file_path="/private/assets/locked-2.bin",
|
|
content_id="content-2",
|
|
record_id="record-2",
|
|
extract_metadata=False,
|
|
compute_hash=False,
|
|
progress=progress,
|
|
)
|
|
|
|
assert first is False
|
|
assert second is False
|
|
assert events_named(caplog, "scanner.stat_failed") == [
|
|
{"error_type": "PermissionError", "site": "enrich"}
|
|
]
|
|
assert progress.permission_denied == 2
|
|
|
|
|
|
def test_missing_file_during_enrichment_returns_false_without_emitting(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
def vanish_stat(*_args, **_kwargs):
|
|
raise FileNotFoundError("/private/assets/gone.bin")
|
|
|
|
monkeypatch.setattr(scanner, "os", SimpleNamespace(stat=vanish_stat, path=scanner.os.path))
|
|
progress = _ScanState()
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
updated = scanner.enrich_asset(
|
|
Mock(),
|
|
file_path="/private/assets/gone.bin",
|
|
content_id="content",
|
|
record_id="record",
|
|
extract_metadata=False,
|
|
compute_hash=False,
|
|
progress=progress,
|
|
)
|
|
|
|
assert updated is False
|
|
assert events_named(caplog, "scanner.stat_failed") == []
|
|
|
|
|
|
def test_enrich_failures_emit_once_per_scan_and_reset_with_new_scan(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
rows = [
|
|
UnenrichedContent("content-1", "record-1", "/private/assets/one.bin"),
|
|
UnenrichedContent("content-2", "record-2", "/private/assets/two.bin"),
|
|
]
|
|
monkeypatch.setattr(scanner, "create_session", lambda: nullcontext(Mock()))
|
|
|
|
def fail_enrich(*_args, **_kwargs):
|
|
raise FileNotFoundError("/private/assets/secret.bin")
|
|
|
|
monkeypatch.setattr(scanner, "enrich_asset", fail_enrich)
|
|
|
|
with caplog.at_level(logging.INFO):
|
|
first_result = scanner.enrich_assets_batch(rows, progress=_ScanState())
|
|
second_result = scanner.enrich_assets_batch(rows[:1], progress=_ScanState())
|
|
|
|
assert first_result == (0, ["record-1", "record-2"])
|
|
assert second_result == (0, ["record-1"])
|
|
assert events_named(caplog, "scanner.enrich_failed") == [
|
|
{"error_type": "FileNotFoundError"},
|
|
{"error_type": "FileNotFoundError"},
|
|
]
|
|
|
|
|
|
def test_enrich_exception_counts_one_failure_per_raising_row(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
rows = [
|
|
UnenrichedContent("content-1", "record-1", "/private/assets/one.bin"),
|
|
UnenrichedContent("content-2", "record-2", "/private/assets/two.bin"),
|
|
]
|
|
monkeypatch.setattr(scanner, "create_session", lambda: nullcontext(Mock()))
|
|
|
|
def fail_enrich(*_args, **_kwargs):
|
|
raise FileNotFoundError("/private/assets/secret.bin")
|
|
|
|
monkeypatch.setattr(scanner, "enrich_asset", fail_enrich)
|
|
progress = _ScanState()
|
|
|
|
enriched, failed_ids = scanner.enrich_assets_batch(rows, progress=progress)
|
|
|
|
assert enriched == 0
|
|
assert failed_ids == ["record-1", "record-2"]
|
|
assert progress.enrich_failed == 2
|
|
|
|
|
|
def test_benign_enrich_no_op_is_skipped_without_counting_a_failure(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
deleted = tmp_path / "gone.safetensors"
|
|
rows = [UnenrichedContent("content-1", "record-1", str(deleted))]
|
|
monkeypatch.setattr(scanner, "create_session", lambda: nullcontext(Mock()))
|
|
progress = _ScanState()
|
|
|
|
enriched, failed_ids = scanner.enrich_assets_batch(rows, progress=progress)
|
|
|
|
assert enriched == 0
|
|
assert failed_ids == ["record-1"]
|
|
assert progress.enrich_failed == 0
|