bvandeusen
f747750f97
The Dashboard's "Retry All Failed" button used to fetch every non-superseded
failure, then fire one retry HTTP call per row. Two missing guards:
- No per-source dedup — a source with N historical failures enqueued N jobs
for the same URL.
- No "recent success" check — a source that succeeded two hours ago could
still have stale un-superseded failures from last week that got re-queued.
Adds POST /api/downloads/retry-failed-bulk:
• Selects non-superseded failures, ordered (source_id ASC, created_at DESC)
• Per-source dedup keeps only the most recent failure per source
• Skips sources whose last_success is within the configurable recent window
(default 24h, via recent_window_hours body param)
• Resets + enqueues in a single DB transaction, single Celery dispatch loop
• Returns a per-reason skip breakdown: duplicate_source, recent_success,
no_source (orphaned rows)
The Dashboard button now calls the bulk endpoint and surfaces the full
skip breakdown in the toast, so the user knows exactly what was (and
wasn't) queued. Logic is extracted into a pure _plan_bulk_retry helper
and unit-tested against SimpleNamespace fixtures — 7 new tests covering
the dedup, window, and orphan paths.
125 lines
4.7 KiB
Python
125 lines
4.7 KiB
Python
"""Unit tests for the planner helper behind POST /downloads/retry-failed-bulk.
|
|
|
|
The helper decides which non-superseded failed downloads get re-queued, with
|
|
per-source dedup and a "recent success" skip window. These tests exercise the
|
|
dedup + filter logic on plain objects (no DB).
|
|
"""
|
|
from datetime import datetime, timedelta, timezone
|
|
from types import SimpleNamespace
|
|
|
|
from app.api.downloads import _plan_bulk_retry
|
|
|
|
|
|
def _failure(failure_id, source_id, created_at):
|
|
return SimpleNamespace(id=failure_id, source_id=source_id, created_at=created_at)
|
|
|
|
|
|
def _source(source_id, last_success):
|
|
return SimpleNamespace(id=source_id, last_success=last_success)
|
|
|
|
|
|
# Tests expect failures already sorted (source_id ASC, created_at DESC) — same
|
|
# ordering the endpoint's SQL query produces.
|
|
|
|
def test_no_failures_returns_empty_plan():
|
|
retry_ids, summary = _plan_bulk_retry([], {}, datetime.now(timezone.utc))
|
|
assert retry_ids == []
|
|
assert summary == {
|
|
"retried": 0,
|
|
"skipped_duplicate_source": 0,
|
|
"skipped_recent_success": 0,
|
|
"skipped_no_source": 0,
|
|
}
|
|
|
|
|
|
def test_single_failure_retried_when_source_never_succeeded():
|
|
now = datetime.now(timezone.utc)
|
|
failures = [_failure(1, source_id=10, created_at=now - timedelta(hours=48))]
|
|
sources = {10: _source(10, last_success=None)}
|
|
|
|
retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24))
|
|
|
|
assert retry_ids == [1]
|
|
assert summary["retried"] == 1
|
|
|
|
|
|
def test_dedup_keeps_only_most_recent_per_source():
|
|
"""Given 3 failures for same source (sorted by created_at DESC), only the first survives."""
|
|
now = datetime.now(timezone.utc)
|
|
failures = [
|
|
_failure(301, source_id=10, created_at=now - timedelta(hours=1)), # most recent
|
|
_failure(302, source_id=10, created_at=now - timedelta(hours=24)),
|
|
_failure(303, source_id=10, created_at=now - timedelta(days=7)),
|
|
]
|
|
sources = {10: _source(10, last_success=None)}
|
|
|
|
retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24))
|
|
|
|
assert retry_ids == [301]
|
|
assert summary["skipped_duplicate_source"] == 2
|
|
|
|
|
|
def test_skip_source_with_recent_success():
|
|
"""Source that succeeded 2 hours ago should be skipped against a 24h window."""
|
|
now = datetime.now(timezone.utc)
|
|
failures = [_failure(1, source_id=10, created_at=now - timedelta(days=7))]
|
|
sources = {10: _source(10, last_success=now - timedelta(hours=2))}
|
|
|
|
retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24))
|
|
|
|
assert retry_ids == []
|
|
assert summary["skipped_recent_success"] == 1
|
|
|
|
|
|
def test_retry_source_with_old_success():
|
|
"""Source that succeeded 48h ago should still retry against a 24h window."""
|
|
now = datetime.now(timezone.utc)
|
|
failures = [_failure(1, source_id=10, created_at=now - timedelta(hours=1))]
|
|
sources = {10: _source(10, last_success=now - timedelta(hours=48))}
|
|
|
|
retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24))
|
|
|
|
assert retry_ids == [1]
|
|
assert summary["skipped_recent_success"] == 0
|
|
|
|
|
|
def test_orphaned_failure_without_source_id_is_skipped_separately():
|
|
"""Failures with source_id=None shouldn't explode — tallied under a distinct bucket."""
|
|
now = datetime.now(timezone.utc)
|
|
failures = [
|
|
_failure(1, source_id=None, created_at=now),
|
|
_failure(2, source_id=10, created_at=now),
|
|
]
|
|
sources = {10: _source(10, last_success=None)}
|
|
|
|
retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24))
|
|
|
|
assert retry_ids == [2]
|
|
assert summary["skipped_no_source"] == 1
|
|
assert summary["skipped_duplicate_source"] == 0
|
|
|
|
|
|
def test_mixed_scenario_combines_all_rules():
|
|
"""One healthy source (recent success, skipped), one broken source (multiple failures, dedup'd)."""
|
|
now = datetime.now(timezone.utc)
|
|
failures = [
|
|
_failure(101, source_id=10, created_at=now - timedelta(hours=1)), # healthy source, skip
|
|
_failure(201, source_id=20, created_at=now - timedelta(hours=2)), # broken, pick this one
|
|
_failure(202, source_id=20, created_at=now - timedelta(days=1)), # broken, dedup
|
|
_failure(203, source_id=20, created_at=now - timedelta(days=7)), # broken, dedup
|
|
]
|
|
sources = {
|
|
10: _source(10, last_success=now - timedelta(hours=3)), # healthy
|
|
20: _source(20, last_success=None), # never succeeded
|
|
}
|
|
|
|
retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24))
|
|
|
|
assert retry_ids == [201]
|
|
assert summary == {
|
|
"retried": 1,
|
|
"skipped_duplicate_source": 2,
|
|
"skipped_recent_success": 1,
|
|
"skipped_no_source": 0,
|
|
}
|