Skip to content

Pin Cloudflare account for R2 publishing and fix publish-raw failures - #148

Merged
juaristi22 merged 1 commit into
mainfrom
pin-r2-account-and-fix-publish
Aug 10, 2026
Merged

Pin Cloudflare account for R2 publishing and fix publish-raw failures#148
juaristi22 merged 1 commit into
mainfrom
pin-r2-account-and-fix-publish

Conversation

@MaxGhenis

Copy link
Copy Markdown
Contributor

What

  • Pin the Cloudflare account in wrangler.toml so every wrangler r2 call made by chronicle publish-raw / publish-derived / fetch-artifact --upload-r2 targets the PolicyEngine account without any environment setup. Non-interactive wrangler cannot select an account when the logged-in user belongs to more than one, and subprocess-driven uploads are always non-interactive — this is exactly the failure multi-account contributors hit, and it reads like a permissions error. Verified empirically that wrangler r2 honors wrangler.toml account_id (a deliberately wrong ID makes it target that wrong account).
  • Let publish-raw handle label-keyed manifest file entries. db/data/ssa/ssi_monthly_statistics_2024_12/manifest.yaml has a source_capture entry (HTML capture alongside the CSV); _publish_raw_manifest_entry crashed on int("source_capture"). build_r2_key only ever stringifies the year segment — its existing stored key already uses the literal source_capture segment — so pass the value through and widen the annotation to int | str. Test added reproducing the SSA shape.
  • Drop --force from wrangler r2 object put. Current wrangler removed the flag ("Unknown argument: force"), so every upload fails for anyone resolving a fresh wrangler — and put overwrites by default anyway (verified on 4.59.2: repeated puts of the same key succeed without the flag).
  • README: document that wrangler login is the only auth step, and swap the npx example for bunx.

Context

R2 storage for ledger-raw / ledger-derived moved to the PolicyEngine Cloudflare account (20d90f557651969925eece96e58e24dc) on 2026-08-07 — the original buckets were bootstrapped on a legacy cosilico-era account in May. All committed raw artifacts (~58 MB) were re-published to the new account; keys are content-addressed, so every manifest is byte-identical and no data files change in this PR.

🤖 Generated with Claude Code

- Add wrangler.toml pinning the PolicyEngine account so non-interactive
  wrangler r2 calls work for contributors whose Cloudflare user belongs
  to multiple accounts.
- Pass manifest file-entry years through to build_r2_key unchanged:
  label entries such as the SSA source_capture capture crashed int(year).
- Drop --force from wrangler r2 object put: current wrangler removed the
  flag and put overwrites by default.
- Document the wrangler login flow in the README.
- census/population_projections_2023: record the artifact's actual R2 key
  (its manifest source_id never matched the historical key convention).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juaristi22

Copy link
Copy Markdown
Collaborator

Independent verification pass (read-only, run locally against both branches):

  • The label-year fix resolves exactly the whole-tree publish-raw crash the UK PR chain documented (int("source_capture") / int("historical_service_source") at artifacts.py), and the added test reproduces the SSA shape faithfully.
  • The census population_projections_2023 manifest key change checks out mechanically: re-deriving keys from ids via build_r2_key across every stored manifest entry reproduces the committed keys 77/78, and the single mismatch is this manifest's old key — the edit here brings it to the derived form. One nit: the PR body says "every manifest is byte-identical and no data files change in this PR", which this edit contradicts — worth a one-line body amendment naming the census key correction so a reviewer doesn't trip on it.
  • --force removal: consistent with current wrangler; our sandbox harness runs are argument-agnostic either way.

Sequencing note: #149's verification used this PR's tooling, and without this PR #149 leaves the census manifest as the one id/key inconsistency — merge this first (or both together). With both in, the per-directory workaround in the UK PRs' publish instructions becomes unnecessary: whole-tree publish-raw --root db/data should cover everything.

🤖 Generated with Claude Code

@juaristi22

Copy link
Copy Markdown
Collaborator

Full merge-readiness review (local merged preview of #148 + #149 on current main) — verdict: ready, merge this before #149.

Code review:

  • The year: int | str pass-through is correct at both build_r2_key call sites; every other caller still passes ints. The added test reproduces the SSA source_capture shape and asserts the exact derived key.
  • --force removal is safe by construction here, not just empirically: keys are content-addressed (.../<sha256>/<filename>), so a same-key overwrite can only ever rewrite identical bytes.
  • One hardening nit (non-blocking): build_r2_key interpolates str(year) without _clean_key_part, so the year segment is now the only unsanitized part of the key — a future label with a space or slash would embed raw. Input is repo-controlled today; worth a one-line _clean_key_part(str(year)) whenever convenient.

Empirical, on the merged preview: tests/test_chronicle_artifacts.py 12/12; whole-tree publish-raw --root db/data under a local wrangler stub scans 57 manifests / 78 artifacts, 0 failures, 0 scan errors — the crash class the UK PRs documented is gone — and the republish reproduced every committed storage key byte-identically (the only manifest diffs were YAML line-rewrap cosmetics). The census key correction in this PR is exactly what the id-derivation produces post-#149.

🤖 Generated with Claude Code

@juaristi22
juaristi22 merged commit 8fb7d25 into main Aug 10, 2026
2 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.

2 participants