refactor(downloads): DRY the ingester/gallery-dl seam — A1–A4 (plan #707)
Consolidates duplication that owning the native ingester left against the still- live gallery-dl path, and fixes a parity gap the duplication hid. A1 — shared quarantine: extract file_validator.quarantine_file (move to _quarantine/<slug>/<platform> + write the .quarantine.json provenance sidecar). gallery_dl._validate_and_quarantine and patreon_downloader._validate_path both call it. PARITY FIX: the native path now writes the provenance sidecar it previously skipped — threads the media url through for source_url. A2 — make_run_stats(**counts) factory in gallery_dl for the canonical run_stats key set; gallery_dl._compute_run_stats and ingest_core both build through it so the shape can't drift (gallery-dl path gains a benign dead_lettered_count=0). A3 — one safe_ext in utils/paths.py; importer._safe_ext (thin wrapper, kept for the Path call sites + memory pointer) and patreon_client both use it. Closes the double-impl of the URL-encoded-basename ext gotcha. A4 — promote gallery_dl._truncate_log/_extract_errors_warnings to module-level truncate_log/extract_errors_warnings; download_service calls them directly instead of reaching through self.gdl for native-result log shaping. The staticmethods stay as thin delegators for existing callers/tests. Behavior-preserving except the A1 sidecar parity fix. Test: native quarantine writes a .quarantine.json (test_patreon_downloader). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -31,7 +31,6 @@ import contextlib
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import time
|
||||
from collections.abc import Callable
|
||||
@@ -42,7 +41,7 @@ from urllib.parse import urlsplit
|
||||
|
||||
import requests
|
||||
|
||||
from .file_validator import is_validatable, validate_file
|
||||
from .file_validator import is_validatable, quarantine_file, validate_file
|
||||
from .patreon_client import (
|
||||
_BACKOFF_CAP_SECONDS,
|
||||
_load_session,
|
||||
@@ -268,7 +267,9 @@ class PatreonDownloader:
|
||||
out_path = Path(out_path)
|
||||
else:
|
||||
out_path = self._fetch_get(media.url, media_path)
|
||||
reason, quarantine_dest = self._validate_path(out_path, artist_slug)
|
||||
reason, quarantine_dest = self._validate_path(
|
||||
out_path, artist_slug, media.url
|
||||
)
|
||||
if reason is not None:
|
||||
# Quarantined (corrupt/invalid) — distinct from a download error
|
||||
# so the run can report a real files_quarantined count + paths.
|
||||
@@ -426,16 +427,16 @@ class PatreonDownloader:
|
||||
# -- validation --------------------------------------------------------
|
||||
|
||||
def _validate_path(
|
||||
self, path: Path, artist_slug: str
|
||||
self, path: Path, artist_slug: str, source_url: str | None = None
|
||||
) -> tuple[str | None, Path | None]:
|
||||
"""Validate a freshly-written file; quarantine if bad.
|
||||
|
||||
Mirrors gallery_dl._validate_and_quarantine: fail-open for unknown
|
||||
formats, move the corrupt file to _quarantine/<slug>/patreon. Returns
|
||||
`(reason, quarantine_dest)` when quarantined (dest is the original path if
|
||||
the move itself failed), else `(None, None)` (ok / not validatable /
|
||||
disabled). plan #704: the dest is surfaced so the run reports a real
|
||||
quarantined-paths list instead of a blank count.
|
||||
Uses the shared `file_validator.quarantine_file` — same move + provenance
|
||||
sidecar gallery-dl writes (the native path used to skip the sidecar; that
|
||||
parity gap is closed here). Returns `(reason, quarantine_dest)` when
|
||||
quarantined (dest is the original path if the move itself failed), else
|
||||
`(None, None)` (ok / not validatable / disabled). plan #704: the dest is
|
||||
surfaced so the run reports a real quarantined-paths list.
|
||||
"""
|
||||
if not self._validate or not is_validatable(path):
|
||||
return None, None
|
||||
@@ -446,20 +447,11 @@ class PatreonDownloader:
|
||||
return None, None
|
||||
if result.ok:
|
||||
return None, None
|
||||
quarantine_root = self.images_root / "_quarantine" / artist_slug / "patreon"
|
||||
dest = path
|
||||
try:
|
||||
quarantine_root.mkdir(parents=True, exist_ok=True)
|
||||
dest = quarantine_root / path.name
|
||||
counter = 1
|
||||
while dest.exists():
|
||||
dest = quarantine_root / f"{path.stem}.{counter}{path.suffix}"
|
||||
counter += 1
|
||||
shutil.move(str(path), str(dest))
|
||||
except OSError as exc:
|
||||
log.error("Failed to quarantine %s: %s. File left in place.", path, exc)
|
||||
dest = path
|
||||
return (result.reason or "validation failed"), dest
|
||||
dest = quarantine_file(
|
||||
self.images_root, path, artist_slug, "patreon",
|
||||
url=source_url, result=result,
|
||||
)
|
||||
return (result.reason or "validation failed"), (dest or path)
|
||||
|
||||
# -- sidecar -----------------------------------------------------------
|
||||
|
||||
|
||||
Reference in New Issue
Block a user