feat: Settings can re-download the Discord images the None naming broke (3999)
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 2s
Build images / build-agent (push) Successful in 6s
CI / frontend-build (push) Successful in 19s
CI / backend-lint-and-test (push) Successful in 33s
Build images / build-web (push) Successful in 1m9s
Build images / smoke-web (push) Skipped
Build images / build-ml (push) Successful in 2m3s
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m35s
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 2s
Build images / build-agent (push) Successful in 6s
CI / frontend-build (push) Successful in 19s
CI / backend-lint-and-test (push) Successful in 33s
Build images / build-web (push) Successful in 1m9s
Build images / smoke-web (push) Skipped
Build images / build-ml (push) Successful in 2m3s
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m35s
The operator chose a clean re-download over relinking in place. The ~1,600 broken files can't be relinked reliably: their message ids are gone, and their sidecars collided.
Settings → Maintenance → "Repair Discord downloads" previews, then applies:
- Deletes every image whose path is `…/discord/None/<8 digits>_None_…`. Both the folder and the name are required, because that pair is only what the bug produced. It reuses cleanup_service.delete_images for the record and file deletes.
- Sweeps the leftover collided sidecars from those folders and removes the emptied folders.
- Only then clears gallery-dl's archive rows `discord%`, excluding `discordasset_%`. Upstream keys message attachments as `discord{message_id}_{num}`. Since the broken files lost their message ids, per-source forgetting is impossible. Every pre-fix Discord download is broken, and files fetched after the fix still exist on disk, so gallery-dl's `skip` won't re-fetch them.
- Arms a fresh backfill on every Discord source.
The apply defaults to preview at both the route and the task, runs on maintenance_long, and is never on a beat. The card uses the confirm-dialog pattern of AttachmentReclaimCard.
Supporting refactors, with no behaviour change:
- gallery_dl.archive_path() is the single definition of the archive location.
- source_service.arm_backfill() is the mutation start_backfill already did, now shared with the sync repair.
Tests (tests/test_discord_repair.py):
- The archive clear leaves other platforms and Discord assets alone, and counting mutates nothing.
- Case-twin artist folders are both found.
- The folder sweep works.
- An integration run shows only the broken image goes. A correctly named Discord file and a `None` folder under Patreon survive, and only Discord sources are re-armed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SHQB1YukL3VyvMK8rcbmV9
This commit is contained in:
@@ -0,0 +1,183 @@
|
||||
"""Repair the Discord downloads made before the naming fix (issue #3999).
|
||||
|
||||
Until dc840fe, gallery-dl's Discord patterns asked for keys the extractor never
|
||||
emits, so every Discord download landed as
|
||||
`<artist>/discord/None/<date>_None_<original name>`, next to a sidecar named
|
||||
after the attachment's ORIGINAL name. That broke two things:
|
||||
|
||||
* **No Post, no date.** `find_sidecar` can never pair those names, so these
|
||||
files were imported as loose images with no Post, and a card shows the
|
||||
download time.
|
||||
* **No trustworthy metadata to relink from.** Every `image.png` in a channel
|
||||
wrote the same `image.json`, so the surviving sidecar describes whichever
|
||||
message was written last. The message id is gone from the filename too.
|
||||
|
||||
The operator chose a clean re-download (2026-09-13) over relinking in place:
|
||||
delete the broken files and their records, make gallery-dl forget it fetched
|
||||
them, and backfill every Discord source again under the fixed naming.
|
||||
|
||||
## Why the archive is cleared for ALL of Discord
|
||||
|
||||
gallery-dl records a download as `discord{message_id}_{num}` (upstream
|
||||
`DiscordExtractor.archive_fmt`, prefixed with the category). The broken files
|
||||
lost their message ids, so there is no way to forget one source's entries and
|
||||
not another's. Every Discord download made before the fix is broken, so
|
||||
forgetting all of them is exactly right. Anything downloaded AFTER the fix still
|
||||
exists on disk under its correct name, and gallery-dl's `skip` sees the file and
|
||||
does not fetch it again.
|
||||
|
||||
## What it does not touch
|
||||
|
||||
Discord posts FC grouped itself (#388 E2) are built from Posts, and these files
|
||||
never had one, so there is nothing grouped to unwind. Images outside a
|
||||
`discord/None/` directory are never selected: the path filter requires both the
|
||||
`None` directory and the `_None_` filename, the pair only the bug produced.
|
||||
|
||||
Operator-triggered only (Settings, preview first). Never on a beat.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from ..models import ImageRecord, Source
|
||||
from .cleanup_service import delete_images
|
||||
from .gallery_dl import archive_path
|
||||
from .source_service import arm_backfill
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
# `%` and `_` are LIKE wildcards, so the literal underscores around None are
|
||||
# escaped. `________` is the eight-digit date prefix the old pattern wrote.
|
||||
_BROKEN_PATH_LIKE = r"%/discord/None/________\_None\_%"
|
||||
|
||||
# Upstream keys asset downloads as `asset_{server_id}_{id}`. FC never fetches
|
||||
# server assets, but excluding them keeps this to exactly the message
|
||||
# attachments the bug mangled.
|
||||
_ARCHIVE_SQL_MATCH = r"entry LIKE 'discord%' AND entry NOT LIKE 'discordasset\_%' ESCAPE '\'"
|
||||
_COUNT_SQL = "SELECT COUNT(*) FROM archive WHERE " + _ARCHIVE_SQL_MATCH
|
||||
_DELETE_SQL = "DELETE FROM archive WHERE " + _ARCHIVE_SQL_MATCH
|
||||
|
||||
|
||||
def broken_directories(images_root: Path) -> list[Path]:
|
||||
"""Every `<artist>/discord/None` directory. Artist folders that differ only by
|
||||
case (`Conto` and `conto`) are separate directories and both are found."""
|
||||
return sorted(d for d in Path(images_root).glob("*/discord/None") if d.is_dir())
|
||||
|
||||
|
||||
def count_archive_entries(archive: Path) -> int:
|
||||
return _archive(archive, delete=False)
|
||||
|
||||
|
||||
def forget_archive_entries(archive: Path) -> int:
|
||||
return _archive(archive, delete=True)
|
||||
|
||||
|
||||
def _archive(archive: Path, *, delete: bool) -> int:
|
||||
if not archive.is_file():
|
||||
return 0
|
||||
# A download running at the same moment holds this file briefly. Waiting
|
||||
# 30s for its lock beats failing the repair over a transient contention.
|
||||
conn = sqlite3.connect(str(archive), timeout=30)
|
||||
try:
|
||||
has_table = conn.execute(
|
||||
"SELECT 1 FROM sqlite_master WHERE type='table' AND name='archive'"
|
||||
).fetchone()
|
||||
if not has_table:
|
||||
return 0
|
||||
if not delete:
|
||||
return conn.execute(_COUNT_SQL).fetchone()[0]
|
||||
cur = conn.execute(_DELETE_SQL)
|
||||
conn.commit()
|
||||
return cur.rowcount
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def _sweep_directory(directory: Path) -> tuple[int, bool]:
|
||||
"""Remove what the record deletes left behind: the collided sidecars, plus any
|
||||
file that never became a record (a quarantined or rejected download). Then
|
||||
the directory itself, if it is empty. Returns (files removed, dir removed)."""
|
||||
removed = 0
|
||||
for f in directory.iterdir():
|
||||
if f.is_file():
|
||||
try:
|
||||
f.unlink()
|
||||
removed += 1
|
||||
except OSError as exc:
|
||||
log.warning("discord repair: could not remove %s: %s", f, exc)
|
||||
try:
|
||||
directory.rmdir()
|
||||
return removed, True
|
||||
except OSError:
|
||||
return removed, False
|
||||
|
||||
|
||||
def repair_discord_downloads(
|
||||
session: Session, *, images_root: Path, dry_run: bool,
|
||||
) -> dict:
|
||||
images_root = Path(images_root)
|
||||
archive = archive_path(images_root)
|
||||
|
||||
broken = select(ImageRecord.id, ImageRecord.size_bytes).where(
|
||||
ImageRecord.path.like(_BROKEN_PATH_LIKE, escape="\\")
|
||||
)
|
||||
rows = session.execute(broken).all()
|
||||
image_ids = [r.id for r in rows]
|
||||
directories = broken_directories(images_root)
|
||||
sources = session.execute(
|
||||
select(Source).where(Source.platform == "discord")
|
||||
).scalars().all()
|
||||
|
||||
summary = {
|
||||
"images": len(image_ids),
|
||||
"bytes": sum(r.size_bytes or 0 for r in rows),
|
||||
"directories": len(directories),
|
||||
"sources": len(sources),
|
||||
"enabled_sources": sum(1 for s in sources if s.enabled),
|
||||
}
|
||||
|
||||
if dry_run:
|
||||
summary["archive_entries"] = count_archive_entries(archive)
|
||||
return summary
|
||||
|
||||
deleted = delete_images(session, image_ids=image_ids, images_root=images_root)
|
||||
|
||||
swept = 0
|
||||
directories_removed = 0
|
||||
for d in directories:
|
||||
n, gone = _sweep_directory(d)
|
||||
swept += n
|
||||
directories_removed += int(gone)
|
||||
|
||||
# Only after the files are gone. Forgetting first and failing half way would
|
||||
# leave gallery-dl free to re-fetch into a directory still full of the old
|
||||
# copies.
|
||||
forgotten = forget_archive_entries(archive)
|
||||
|
||||
for source in sources:
|
||||
arm_backfill(source)
|
||||
session.commit()
|
||||
|
||||
remaining = session.execute(
|
||||
select(func.count(ImageRecord.id)).where(
|
||||
ImageRecord.path.like(_BROKEN_PATH_LIKE, escape="\\")
|
||||
)
|
||||
).scalar_one()
|
||||
|
||||
summary.update(
|
||||
images_deleted=deleted["images_deleted"],
|
||||
files_failed=deleted["files_failed"],
|
||||
leftover_files_removed=swept,
|
||||
directories_removed=directories_removed,
|
||||
archive_entries=forgotten,
|
||||
backfills_started=len(sources),
|
||||
remaining=remaining,
|
||||
)
|
||||
log.info("discord repair applied: %s", summary)
|
||||
return summary
|
||||
@@ -136,6 +136,16 @@ def metadata_postprocessor(filename: str) -> dict:
|
||||
return {"name": "metadata", "mode": "json", "directory": ".", "filename": filename}
|
||||
|
||||
|
||||
def archive_path(images_root: Path) -> Path:
|
||||
"""gallery-dl's download archive: the record of what it has already fetched.
|
||||
|
||||
One definition, because the Discord repair (services/discord_repair.py) has
|
||||
to find the same file the downloader writes, without constructing a service
|
||||
whose __init__ creates directories.
|
||||
"""
|
||||
return Path(images_root) / ".gallery-dl" / "archive.sqlite3"
|
||||
|
||||
|
||||
@dataclass
|
||||
class SourceConfig:
|
||||
"""Per-source overrides loaded from Source.config_overrides JSON.
|
||||
@@ -447,7 +457,7 @@ class GalleryDLService:
|
||||
config = {
|
||||
"extractor": {
|
||||
"base-directory": str(self.images_root),
|
||||
"archive": str(self._config_dir / "archive.sqlite3"),
|
||||
"archive": str(archive_path(self.images_root)),
|
||||
"skip": True,
|
||||
"sleep": self._rate_limit,
|
||||
"sleep-request": max(0.5, self._rate_limit / 4),
|
||||
|
||||
@@ -158,6 +158,19 @@ def _is_app_managed(key: str) -> bool:
|
||||
BACKFILL_MAX_CHUNKS = 200
|
||||
|
||||
|
||||
def arm_backfill(source: Source) -> None:
|
||||
"""Arm a fresh run-until-done backfill on `source` (plan #693). Mutation
|
||||
only — the caller commits. Shared by `SourceService.start_backfill` and the
|
||||
sync Discord repair, so both clear exactly the same resume state."""
|
||||
co = dict(source.config_overrides or {})
|
||||
co["_backfill_state"] = "running"
|
||||
for k in ("_backfill_cursor", "_backfill_cursor_stalls", "_backfill_chunks",
|
||||
"_backfill_posts"):
|
||||
co.pop(k, None)
|
||||
source.config_overrides = co
|
||||
source.backfill_runs_remaining = BACKFILL_MAX_CHUNKS
|
||||
|
||||
|
||||
class SourceService:
|
||||
def __init__(self, session: AsyncSession):
|
||||
self.session = session
|
||||
@@ -473,13 +486,7 @@ class SourceService:
|
||||
)).scalar_one_or_none()
|
||||
if source is None:
|
||||
raise LookupError(f"source id={source_id} not found")
|
||||
co = dict(source.config_overrides or {})
|
||||
co["_backfill_state"] = "running"
|
||||
for k in ("_backfill_cursor", "_backfill_cursor_stalls", "_backfill_chunks",
|
||||
"_backfill_posts"):
|
||||
co.pop(k, None)
|
||||
source.config_overrides = co
|
||||
source.backfill_runs_remaining = BACKFILL_MAX_CHUNKS
|
||||
arm_backfill(source)
|
||||
await self.session.commit()
|
||||
return await self._row_to_record(source)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user