feat(patreon): structured ingester results + quarantine surfacing — #704 step 1
The native ingester faked gallery-dl stdout (`Cursor:` lines, summary) and phase 3 regex-scraped it back — so Patreon run-stats were mostly zero and quarantine stats blank. We own the ingester, so it now RETURNS structured data and phase 3 reads it directly. - DownloadResult gains run_stats/cursor/posts_processed (None/0 on the gallery-dl path, which keeps the text route). - Ingester builds real run_stats from per-media outcome counts, sets the checkpoint cursor structurally (no fake `Cursor:` stdout), and counts posts processed. download_service phase 3 uses dl_result.run_stats when present; the backfill lifecycle + TIMEOUT→PARTIAL block checkpoint dl_result.cursor instead of parse_last_cursor(stdout). - #4 quarantine: PatreonDownloader reports a distinct "quarantined" MediaOutcome (with the _quarantine dest); the ingester surfaces a real files_quarantined + quarantined_paths + run_stats.quarantined_count (was hardcoded 0). Quarantined media isn't written or marked seen. - Cleanup: parse_last_cursor + _CURSOR_RE (and the now-unused `import re`) removed from gallery_dl — the structured cursor replaced the scrape. Tests: ingester result carries real run_stats/cursor/posts_processed + quarantine counts; downloader quarantines an invalid file as "quarantined"; backfill cursor tests pass cursor= structurally; dropped the parse_last_cursor tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -33,7 +33,6 @@ from .gallery_dl import (
|
||||
ErrorType,
|
||||
GalleryDLService,
|
||||
SourceConfig,
|
||||
parse_last_cursor,
|
||||
)
|
||||
from .importer import Importer
|
||||
from .patreon_ingester import PatreonIngester
|
||||
@@ -155,7 +154,7 @@ class DownloadService:
|
||||
# timed out with NO progress stays TIMEOUT and feeds phase 3's
|
||||
# stall-guard. Leave RATE_LIMITED alone so the platform-cooldown fires.
|
||||
if in_backfill and dl_result.error_type == ErrorType.TIMEOUT:
|
||||
new_cursor = parse_last_cursor(dl_result.stdout, dl_result.stderr)
|
||||
new_cursor = dl_result.cursor # plan #704: structured, not scraped
|
||||
advanced = bool(
|
||||
(new_cursor and new_cursor != overrides.get("_backfill_cursor"))
|
||||
or dl_result.files_downloaded > 0
|
||||
@@ -439,9 +438,14 @@ class DownloadService:
|
||||
select(DownloadEvent).where(DownloadEvent.id == event_id)
|
||||
)).scalar_one()
|
||||
|
||||
run_stats = self.gdl._compute_run_stats(
|
||||
dl_result.return_code, dl_result.stdout, dl_result.stderr
|
||||
)
|
||||
# plan #704: the native ingester returns structured run_stats; only the
|
||||
# gallery-dl path needs the regex-over-stdout reconstruction.
|
||||
if dl_result.run_stats is not None:
|
||||
run_stats = dict(dl_result.run_stats)
|
||||
else:
|
||||
run_stats = self.gdl._compute_run_stats(
|
||||
dl_result.return_code, dl_result.stdout, dl_result.stderr
|
||||
)
|
||||
run_stats["quarantined_count"] = dl_result.files_quarantined
|
||||
stderr_summary = self.gdl._extract_errors_warnings(dl_result.stderr)
|
||||
|
||||
@@ -528,12 +532,12 @@ class DownloadService:
|
||||
src.backfill_runs_remaining = 0
|
||||
return
|
||||
|
||||
# Did not finish. Patreon checkpoints + resumes via cursor; other
|
||||
# platforms have no resumable cursor (every chunk re-walks from the
|
||||
# top), so they advance only by the download archive growing.
|
||||
# Did not finish. The native ingester checkpoints + resumes via cursor
|
||||
# (carried structurally on the result, plan #704); gallery-dl platforms
|
||||
# have no resumable cursor (every chunk re-walks from the top), so they
|
||||
# advance only by the download archive growing.
|
||||
new_cursor = (
|
||||
parse_last_cursor(dl_result.stdout, dl_result.stderr)
|
||||
if uses_native_ingester(ctx["platform"]) else None
|
||||
dl_result.cursor if uses_native_ingester(ctx["platform"]) else None
|
||||
)
|
||||
advanced = bool(
|
||||
(new_cursor and new_cursor != old_cursor)
|
||||
|
||||
@@ -13,7 +13,6 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
@@ -156,6 +155,14 @@ class DownloadResult:
|
||||
duration_seconds: float = 0.0
|
||||
started_at: str | None = None
|
||||
completed_at: str | None = None
|
||||
# Plan #704 — structured fields the NATIVE ingester populates directly (None
|
||||
# on the gallery-dl path, which keeps the regex-over-stdout route). When set,
|
||||
# phase 3 reads these instead of scraping the text it would otherwise have to
|
||||
# reconstruct: `run_stats` mirrors _compute_run_stats' shape; `cursor` is the
|
||||
# backfill checkpoint the ingester knows exactly (no parse_last_cursor).
|
||||
run_stats: dict | None = None
|
||||
cursor: str | None = None
|
||||
posts_processed: int = 0
|
||||
|
||||
|
||||
def _summarize_validation_failures(failures: list[dict]) -> str:
|
||||
@@ -172,20 +179,9 @@ def _summarize_validation_failures(failures: list[dict]) -> str:
|
||||
return f"{n} files quarantined ({top_count}× {top_reason}, mixed)"
|
||||
|
||||
|
||||
# The native Patreon ingester emits its pagination cursor as `Cursor: <token>`
|
||||
# — one line per fetched page — into the DownloadResult stdout (mirroring the
|
||||
# convention gallery-dl used before the #697 cutover, so the backfill lifecycle
|
||||
# stayed unchanged). The LAST such line is the furthest-progressed page, i.e.
|
||||
# the resume point. We scan both streams (be defensive) and take the final
|
||||
# match; download_service checkpoints it as the next chunk's resume_cursor.
|
||||
# Survives a time-boxed chunk too: the ingester emits the cursor for a page when
|
||||
# it STARTS it, so an interrupted walk still yields its last cursor.
|
||||
_CURSOR_RE = re.compile(r"Cursor:\s*(\S+)")
|
||||
|
||||
|
||||
def parse_last_cursor(stdout: str, stderr: str) -> str | None:
|
||||
matches = _CURSOR_RE.findall(f"{stdout or ''}\n{stderr or ''}")
|
||||
return matches[-1] if matches else None
|
||||
# (parse_last_cursor was removed in plan #704: the native ingester now carries
|
||||
# its checkpoint cursor as a structured DownloadResult.cursor field, so there is
|
||||
# no log text to scrape — and gallery-dl platforms never had a cursor.)
|
||||
|
||||
|
||||
class GalleryDLService:
|
||||
|
||||
@@ -124,11 +124,12 @@ def _post_dir_name(post: dict) -> str:
|
||||
class MediaOutcome:
|
||||
"""Per-media result of a download_post pass.
|
||||
|
||||
status is one of: "downloaded", "skipped_seen", "skipped_disk", "error".
|
||||
`path` is the final on-disk path for "downloaded" (the actual yt-dlp output
|
||||
for video), or the path that already existed for "skipped_disk"; None for
|
||||
"skipped_seen" and (usually) "error". `error` carries the failure reason for
|
||||
"error", else None.
|
||||
status is one of: "downloaded", "skipped_seen", "skipped_disk",
|
||||
"quarantined", "error". `path` is the final on-disk path for "downloaded"
|
||||
(the actual yt-dlp output for video), the path that already existed for
|
||||
"skipped_disk", or the _quarantine destination for "quarantined"; None for
|
||||
"skipped_seen" and (usually) "error". `error` carries the failure/validation
|
||||
reason for "error"/"quarantined", else None.
|
||||
"""
|
||||
|
||||
media: object # MediaItem (avoid importing the name for a bare annotation)
|
||||
@@ -253,10 +254,13 @@ class PatreonDownloader:
|
||||
out_path = Path(out_path)
|
||||
else:
|
||||
out_path = self._fetch_get(media.url, media_path)
|
||||
invalid = self._validate_path(out_path, artist_slug)
|
||||
if invalid is not None:
|
||||
reason, quarantine_dest = self._validate_path(out_path, artist_slug)
|
||||
if reason is not None:
|
||||
# Quarantined (corrupt/invalid) — distinct from a download error
|
||||
# so the run can report a real files_quarantined count + paths.
|
||||
return MediaOutcome(
|
||||
media=media, status="error", path=None, error=invalid
|
||||
media=media, status="quarantined",
|
||||
path=quarantine_dest, error=reason,
|
||||
)
|
||||
|
||||
self._write_sidecar(post, out_path)
|
||||
@@ -358,23 +362,29 @@ class PatreonDownloader:
|
||||
|
||||
# -- validation --------------------------------------------------------
|
||||
|
||||
def _validate_path(self, path: Path, artist_slug: str) -> str | None:
|
||||
"""Validate a freshly-written file; quarantine + return reason if bad.
|
||||
def _validate_path(
|
||||
self, path: Path, artist_slug: str
|
||||
) -> 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, and return
|
||||
the failure reason string (None when ok / not validatable / disabled).
|
||||
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.
|
||||
"""
|
||||
if not self._validate or not is_validatable(path):
|
||||
return None
|
||||
return None, None
|
||||
try:
|
||||
result = validate_file(path)
|
||||
except Exception as exc:
|
||||
log.warning("Validator raised on %s: %s", path, exc)
|
||||
return None
|
||||
return None, None
|
||||
if result.ok:
|
||||
return None
|
||||
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
|
||||
@@ -385,7 +395,8 @@ class PatreonDownloader:
|
||||
shutil.move(str(path), str(dest))
|
||||
except OSError as exc:
|
||||
log.error("Failed to quarantine %s: %s. File left in place.", path, exc)
|
||||
return result.reason or "validation failed"
|
||||
dest = path
|
||||
return (result.reason or "validation failed"), dest
|
||||
|
||||
# -- sidecar -----------------------------------------------------------
|
||||
|
||||
|
||||
@@ -156,8 +156,12 @@ class PatreonIngester:
|
||||
start = time.monotonic()
|
||||
log_lines: list[str] = []
|
||||
written: list[str] = []
|
||||
quarantined_paths: list[str] = []
|
||||
downloaded = 0
|
||||
errors = 0
|
||||
quarantined = 0
|
||||
skipped_count = 0
|
||||
posts_processed = 0
|
||||
consecutive_seen = 0
|
||||
emitted_cursor: str | None = None
|
||||
reached_bottom = False
|
||||
@@ -168,14 +172,17 @@ class PatreonIngester:
|
||||
*, success: bool, return_code: int,
|
||||
error_type: ErrorType | None, error_message: str | None,
|
||||
) -> DownloadResult:
|
||||
# plan #704: return STRUCTURED data — phase 3 reads run_stats/cursor
|
||||
# directly instead of regex-scraping a reconstructed stdout. stdout
|
||||
# stays a human-readable summary (no fake `Cursor:` lines).
|
||||
return DownloadResult(
|
||||
success=success,
|
||||
url=url,
|
||||
artist_slug=artist_slug,
|
||||
platform="patreon",
|
||||
files_downloaded=downloaded,
|
||||
files_quarantined=0,
|
||||
quarantined_paths=[],
|
||||
files_quarantined=quarantined,
|
||||
quarantined_paths=list(quarantined_paths),
|
||||
written_paths=written,
|
||||
stdout="\n".join(log_lines),
|
||||
stderr="",
|
||||
@@ -183,18 +190,28 @@ class PatreonIngester:
|
||||
error_type=error_type,
|
||||
error_message=error_message,
|
||||
duration_seconds=time.monotonic() - start,
|
||||
cursor=emitted_cursor,
|
||||
posts_processed=posts_processed,
|
||||
run_stats={
|
||||
"exit_code": return_code,
|
||||
"downloaded_count": downloaded,
|
||||
"skipped_count": skipped_count,
|
||||
"per_item_failures": errors,
|
||||
"warning_count": 0,
|
||||
"tier_gated_count": 0,
|
||||
"quarantined_count": quarantined,
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
for post, included, page_cursor in self.client.iter_posts(
|
||||
campaign_id, cursor=resume_cursor
|
||||
):
|
||||
# Checkpoint: emit the cursor that FETCHED this page once, the
|
||||
# moment we START it — so a chunk cut mid-page resumes the page,
|
||||
# not the one after it (matches the gallery-dl cursor semantics
|
||||
# download_service's lifecycle already depends on).
|
||||
# Checkpoint the cursor that FETCHED this page the moment we
|
||||
# START it — so a chunk cut mid-page resumes the page, not the one
|
||||
# after it. Carried as DownloadResult.cursor (plan #704); no fake
|
||||
# `Cursor:` stdout line to regex back out.
|
||||
if page_cursor and page_cursor != emitted_cursor:
|
||||
log_lines.append(f"Cursor: {page_cursor}")
|
||||
emitted_cursor = page_cursor
|
||||
|
||||
# Time-box check at the post boundary (coarse, like a gallery-dl
|
||||
@@ -203,6 +220,7 @@ class PatreonIngester:
|
||||
budget_hit = True
|
||||
break
|
||||
|
||||
posts_processed += 1
|
||||
media = self.client.extract_media(post, included)
|
||||
if not media:
|
||||
continue
|
||||
@@ -236,9 +254,20 @@ class PatreonIngester:
|
||||
# do NOT re-feed it to phase 3 — attach_in_place would see
|
||||
# the duplicate sha256 and unlink the on-disk copy.
|
||||
to_mark.append((key, media_item.post_id))
|
||||
skipped_count += 1
|
||||
consecutive_seen += 1
|
||||
elif outcome.status == "skipped_seen":
|
||||
skipped_count += 1
|
||||
consecutive_seen += 1
|
||||
elif outcome.status == "quarantined":
|
||||
# New content that failed validation (corrupt) — counted
|
||||
# distinctly so the run surfaces a real quarantined total.
|
||||
# Not marked seen (a later walk may re-fetch a fixed file);
|
||||
# it IS new content, so it breaks the run-of-seen.
|
||||
quarantined += 1
|
||||
if outcome.path is not None:
|
||||
quarantined_paths.append(str(outcome.path))
|
||||
consecutive_seen = 0
|
||||
elif outcome.status == "error":
|
||||
errors += 1
|
||||
# An error neither advances nor resets the run-of-seen.
|
||||
@@ -262,9 +291,12 @@ class PatreonIngester:
|
||||
|
||||
if errors:
|
||||
log_lines.append(f"{errors} media item(s) failed")
|
||||
if quarantined:
|
||||
log_lines.append(f"{quarantined} media item(s) quarantined (invalid)")
|
||||
log_lines.append(
|
||||
f"Patreon ingest ({mode}): {downloaded} downloaded, "
|
||||
f"{errors} error(s)"
|
||||
f"{skipped_count} skipped, {quarantined} quarantined, "
|
||||
f"{errors} error(s), {posts_processed} post(s)"
|
||||
+ (", reached end" if reached_bottom else "")
|
||||
+ (", time-boxed" if budget_hit else "")
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user