Skip to content

Persist scryfall_cache volume and fail loud on a missing cache - #412

Merged
WilfordGrimley merged 1 commit into
masterfrom
fix-402-scryfall-cache-persistence
Jul 24, 2026
Merged

Persist scryfall_cache volume and fail loud on a missing cache#412
WilfordGrimley merged 1 commit into
masterfrom
fix-402-scryfall-cache-persistence

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

Closes #402. The Scryfall bulk-data cache (MPCAutofill/scryfall_cache/default_cards.json, ~558MB) lived as a plain file inside the django/worker container filesystem with no persistent volume — every image rebuild silently destroyed it. Deploy-2 (2026-07-23) did exactly that, and local_calculate_verdicts went on to run to COMPLETED with a silently degraded (empty) back-face lookup, no error anywhere.

Two independent fixes, both requested by the issue:

  1. Persistent volume: docker/docker-compose.prod.yml gains a named scryfall_cache volume (mirroring the existing postgres_data/elasticsearch_data pattern), mounted on both django (manual exec-run commands) and worker (the weekly import_canonical_card_data/update_dfcs qcluster schedule also reads/refreshes this same file — confirmed via mtg.py's get_canonical_cards_and_artists). The dev compose (docker/docker-compose.yml) only defines the postgres/elasticsearch services, not django/worker, so it doesn't have this hazard and needed no change.
  2. Fail-loud guard: printing_metadata_import.ensure_scryfall_cache_present() is a new existence-only check (deliberately independent of the existing weekly _is_stale staleness/re-download logic, which is untouched), called at the very start of local_calculate_verdicts's Command.handle() before any card-by-card work. Raises a CommandError naming the missing path unless --allow-missing-scryfall-cache is passed explicitly — a message distinct from get_back_face_names's existing soft "empty set" per-card path, which stays as-is (still the right behaviour for a single lookup).

Deploy-time activation note: the compose change only takes effect at the next up --build -d deploy — it does not retroactively attach to the currently-running mpcautofill_django/mpcautofill_worker containers. No containers were rebuilt or restarted from this session.

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate.
  • I have manually tested my changes as follows:
    • docker compose -f docker-compose.prod.yml config --quiet validates the compose file cleanly (no volume/syntax errors).
    • New tests: TestScryfallCacheGuard (5 tests) covering the guard directly (missing/present file) and wired through the management command (refuses without the flag, proceeds with the flag, proceeds when the cache is genuinely present). The 4 pre-existing TestCommandLedgerHardeningAndDryRunGuard tests were updated to pass --allow-missing-scryfall-cache since the test environment has no real cache file on disk (they're testing the dry-run guard, not this one).
    • Full backend suite: 1904 passed, 4 skipped (the 4 skips are the pre-existing documented named skips) via pytest cardpicker in /home/ubuntu/.venvs/mpcautofill-pilot.
    • pre-commit run (ruff/isort/black/mypy/prettier) clean on the final commit; docs_lint.py clean.
  • I have updated any relevant documentation or created new documentation where appropriate.
    • docs/troubleshooting.md: new symptom-first entry for this incident (silently degraded back-face lookup after a rebuild), covering both the cause and the two-part fix, so a future occurrence (e.g. a box rebuild that skips the compose file) is diagnosable quickly.

Notes for reviewer

  • No live containers were touched — this is code + compose-file only. The persistent volume starts empty on its first mount; the next import_scryfall_printing_metadata/import_canonical_card_data run (already scheduled weekly via worker's qcluster) will populate it, same as any fresh bootstrap.
  • No migration involved; checked docs/troubleshooting.md/docs/infrastructure.md for an active migration freeze per CLAUDE.md's process — none found, and N/A here regardless (no schema change).

@WilfordGrimley
WilfordGrimley merged commit f8b56bf into master Jul 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scryfall_cache is lost on every container rebuild

1 participant