Skip to content

Upload raw files above 300 MiB to R2 through its S3 API, and register UCL's house price per square metre (#409, #408 item 6) - #426

Merged
juaristi22 merged 3 commits into
mainfrom
ucl-house-price-per-sqm-409
Nov 9, 2026
Merged

juaristi22 merged 3 commits into
mainfrom
ucl-house-price-per-sqm-409

Conversation

@juaristi22

@juaristi22 juaristi22 commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

This implements option (a) of #409 and uses it for item 6 of #408. Files above Wrangler's 300 MiB limit now go to R2 through its S3-compatible API. UCL's house price per square metre deposit, both bundles, is registered raw-only and is in R2. Max agreed to keeping both bundles in full, not bundle 1 alone.

Large files to R2 (#409, option a)

  • chronicle/r2_s3.py signs requests to R2's S3 API with AWS Signature Version 4 (region auto). It uploads a file as a multipart upload in 100 MiB parts:
    • each part is retried on its own, up to four times;
    • if a part still fails, the upload is aborted, so no partial object is left under the key;
    • only one part is in memory at a time.
  • _upload_r2_object sends a file above WRANGLER_PUT_LIMIT_BYTES (300 MiB) through that path, and everything else through wrangler r2 object put as before. publish-raw, fetch-artifact and publish-derived all upload through it.
  • A raw or fetched file already in R2 is not sent again (Vahid's review). Their keys carry the file's sha256, so _upload_r2_object_in_parts first sends HEAD for the key and skips the upload when the object has the file's size. After a failed upload, such as a completion that committed but lost its response, the same check counts the upload once the object is there, and otherwise the upload's own error is reported. A derived file's key names its build, not its bytes, so publish-derived always sends the file, as Wrangler does below the limit.
  • Credentials.
    • R2_ACCESS_KEY_ID and R2_SECRET_ACCESS_KEY are used when both are set.
    • Otherwise the CLOUDFLARE_API_TOKEN that Wrangler already uses: Cloudflare's R2 token docs give its S3 key ID as the token's ID, read from the token verify endpoint, and its secret as the SHA-256 of its value.
    • The account comes from CLOUDFLARE_ACCOUNT_ID, or else the account_id in wrangler.toml. No new secret is needed.
  • No file is read whole (Vahid's point 1 on Raw files over 300 MiB: how Chronicle should hold UCL's house price per square metre file (chronicle#408, item 6) #409). publish-raw, inventory-artifacts and publish-derived hash with the existing _hash_stream. fetch-artifact streams a download to a .partial file and renames it only when it is complete, so a failed download leaves an earlier copy in place.
  • One part in memory, measured. The first real upload peaked at a 4.8 GB memory footprint, not one part. httpx keeps each response in a reference cycle with its request, which holds the part it sent, so a part stayed in memory until Python's cycle collector ran, and it rarely did. put_file now frees each part and collects before it reads the next:
    • against a fake R2, a 1 GiB upload in 50 MiB parts peaks at 83 MB, down from 1.1 GB;
    • a test that turns the collector off fails without the fix (9.6 MB for eight 1 MiB parts);
    • a full collection takes about 30 ms with Chronicle loaded, once per 100 MiB part.
  • verify-raw is unchanged. It already streams objects back through wrangler r2 object get --pipe, and Wrangler's remote get pipes the API response to stdout without buffering it. It read both bundles back in full and matched their sha256s: 3.2 GB, and 4.4 GB, which is past 4 GiB. Wrangler peaked at 543 MiB resident and verify-raw itself at 170 MB, so Wrangler's get does handle files this size (Vahid's question on Raw files over 300 MiB: how Chronicle should hold UCL's house price per square metre file (chronicle#408, item 6) #409). The first read of bundle 2 was cut when the machine running it went to sleep. verify-raw reported that as fetch_error, not as a mismatch, and a rerun on that file passed.

UCL's house price per square metre (#408, item 6)

ucl-house-price-per-square-metre-856542 (db/data/ucl/house_price_per_square_metre_856542/) registers UK Data Service ReShare 856542, House Price Per Square Metre in England and Wales, 1995-2023, by Bin Chi, Adam Dennett, Thomas Oléron-Evans and Robin Morphet (UCL). It was published on 22 August 2025 under CC BY 4.0 (DOI 10.5255/UKDA-SN-856542). It parses nothing.

  • Bundle 1, 856542_bundle_1.zip: 3,214,161,126 bytes, sha256 e397cdac…34b296. It holds the linked file tranall_link_02122023.zip, with UCL's readme and licence note. The readme describes the linked file as 21,438,854 transactions in England and Wales from 1 January 1995 to 31 October 2023, with 107 fields.
  • Bundle 2, 856542_bundle_2.zip: 4,426,805,029 bytes, sha256 61cbf6a7…02da97. It holds the files UCL linked from:
    • the domestic EPCs downloaded on 2 December 2023, with their address fields removed (epc20_id.zip);
    • Price Paid with field names as at 2 December 2023 (LRPPD_02122023.zip);
    • the November 2023 NSPL (NSPL_NOV_2023_UK.zip).
  • Pinned. ReShare's ETags equal the MD5s of the bytes fetched on 6 November 2026. The new test pins each bundle's sha256, size and MD5, and its canonical key under raw/uk/ucl/.
  • In R2. publish-raw uploaded both through the new path, in 31 and 43 parts, in 2 h 30 min on this connection. R2's multipart ETags, "7eb31fba…44b1df-31" and "d04abcef…798c87-43", equal the MD5 of the parts' MD5s computed from the pinned bytes, and verify-raw matched both sha256s on a full read.
  • CI never downloads them. The registration is raw-only, which r2_only_package_artifacts skips. A test checks that a checkout without the zips stays valid.
  • Licence. ReShare states CC BY 4.0. UCL's licence note adds that the linked file, Price Paid and the NSPL carry Royal Mail address data, limited to non-commercial use, with everything else under the Open Government Licence v3.0. It also quotes the EPC terms: the data contains personal data, and Royal Mail's notice must go with any address data that is shared. As with Price Paid, the bytes stay out of this public repository: the zips are git-ignored and R2 holds them, bundle 1 with that notice.
  • Not registered: the London Datastore's 2024 edition, whose record states no licence (Raw files over 300 MiB: how Chronicle should hold UCL's house price per square metre file (chronicle#408, item 6) #409).

For the reviewers

Tests

  • 342 passed and 3 skipped (counted from JUnit) over the touched surface and the tests that walk db/data:
    • tests/test_chronicle_r2_s3.py (20):
      • Signature Version 4 against AWS's three S3 examples, and canonical query encoding;
      • part order and completion, a retried part, an abort on a 403 or a persistent 503, an <Error> inside a 200, and an empty file;
      • freeing each part, HEAD for a present, missing or refused key, and the credential paths.
    • tests/test_chronicle_artifacts.py (56) and tests/test_chronicle_verify_raw.py (37):
      • the dispatch above the limit, and a failed S3 upload recording no R2 location;
      • hashing with Path.read_bytes patched to fail, and a failed download keeping the earlier copy;
      • an object already under the key, a completion that landed after NoSuchUpload, a derived file sent whatever is under its key, and sparse files of exactly 300 MiB and one byte more.
    • tests/test_source_ucl_house_price_per_square_metre_856542.py (2).
    • The walkers: test_chronicle_boundaries, test_chronicle_geography_names, test_etl_eurostat, test_issue_266_geography_names, test_issue_343_uk_high_value_housing, test_issue_356_local_energy_bus, test_issue_376_one_label_per_value, test_issue_412_uk_area_inputs and test_source_uk_408_facts.
    • The 3 skips are High-value housing facts for the UK local calibration: HM Land Registry Price Paid Data, UK HPI, ONS house price statistics by constituency, the National Statistics Postcode Lookup, and the OBR High Value Council Tax Surcharge costing #343's, which need Price Paid and UK HPI bytes that live in R2.
  • ruff format and ruff check (0.14.10) are clean on the edited files.
  • Against the real bucket: a 12 MiB three-part round trip, then both bundles as above.

🤖 Generated with Claude Code

… UCL's house price per square metre (#409, #408 item 6)

wrangler r2 object put refuses objects above 300 MiB. chronicle/r2_s3.py signs
requests to R2's S3-compatible API (Signature Version 4, region auto) and sends
such files as multipart uploads in 100 MiB parts. A failed part is retried on
its own, and the upload is aborted if a part still fails. Each part is freed,
and the reference cycle httpx leaves around its request collected, before the
next is read, so memory holds one part. Credentials come from R2_ACCESS_KEY_ID
and R2_SECRET_ACCESS_KEY, or else from the CLOUDFLARE_API_TOKEN Wrangler uses.

publish-raw, inventory-artifacts and publish-derived hash files a chunk at a
time, and fetch-artifact streams a download to a .partial file.

UCL's House Price Per Square Metre in England and Wales, 1995-2023 (UK Data
Service ReShare 856542, CC BY 4.0) is registered raw-only, with both bundles
(3.2 and 4.4 GB) in R2 and git-ignored here. No build parses it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vahid-ahmadi

Copy link
Copy Markdown

Review of e120b65. I read chronicle/r2_s3.py, the chronicle/artifacts.py changes, the new tests and the UCL manifest. I ran the PR's four test files (107 passed) in a fresh clone with no Cloudflare credentials set, and ran two probes: one against a mocked transport, one with the upload calls stubbed. I checked ReShare 856542 with HEAD requests and its record page, and Wrangler's size check in cloudflare/workers-sdk. Nothing was uploaded or downloaded in full, so I did not check the readme's description of the measure inside bundle 1.

Should

  1. A re-run, or a completion whose response is lost, sends the whole file again. _publish_raw_manifest_entry calls _upload_r2_object whenever the file is present locally (chronicle/artifacts.py:1702-1706), and _upload_r2_object goes straight to the multipart path above the limit (chronicle/artifacts.py:1525-1526), with no check that the object is already there. The skip at chronicle/artifacts.py:1615 applies only when the zip is absent from the checkout. With Wrangler's 300 MiB files that cost little. Here it is the 2 h 30 min upload again. The same happens after a lost completion. In a probe, the first CompleteMultipartUpload committed and then timed out. The retry (chronicle/r2_s3.py:316-344) got 404 NoSuchUpload, put_file aborted (chronicle/r2_s3.py:238-240), and publish-raw reported r2_upload_failed with the object already in R2. The key already contains the sha256 (build_r2_key, chronicle/artifacts.py:1346-1353), so the same key means the same bytes. Overwriting is not a risk, only time. Fix: in _upload_r2_object_in_parts, send HEAD for the key first and skip the upload when Content-Length equals the file's size. Run the same HEAD before reporting a failed completion. Add one test for each.

Nit

  1. No test at the boundary. The dispatch tests set the limit to 4 bytes and upload a 15-byte file (tests/test_chronicle_artifacts.py:733-751). The code is right: a sparse file of exactly 314,572,800 bytes went to Wrangler, and one byte more went to the S3 path. That matches Wrangler's own sizeBytes > MAX_UPLOAD_SIZE_BYTES check. A test at limit and limit + 1 would pin it.
  2. Two end dates. The provenance notes give the readme's 31 October 2023 (db/data/ucl/house_price_per_square_metre_856542/manifest.yaml:56-57). ReShare's record gives temporal coverage from 1 January 1995 to 23 October 2023. It would be worth noting the record's date next to the readme's.

Checked and fine

  • The threshold is > 300 MiB in binary units (chronicle/artifacts.py:35, :1525), the same as Wrangler's.
  • Parts are a fixed 100 MiB, with only the last part shorter. That is within R2's 5 MiB minimum and 5 GiB maximum, and its rule that parts be of equal size. 31 and 43 parts are far below 10,000.
  • Each part is retried on its own, on 429/5xx and transport errors, and re-signed on each try. A 403 fails at once. Any failure, including an <Error> in a 200 completion, aborts the upload.
  • Each part's body hash is signed as x-amz-content-sha256. The pin's sha256 comes from the whole file, the ETag is never used as a content hash, and verify-raw checks the sha256 end to end.
  • Credentials come only from env vars or wrangler.toml. The secret has repr=False and is not logged, and error messages hold only the method, the key, and the response body or transport error. No secrets are committed.
  • Files at or below the limit still go through wrangler r2 object put unchanged.
  • Hashing and fetch-artifact downloads now stream, and a failed download leaves the earlier copy in place.
  • ReShare 856542: both bundles' Content-Length and ETag (MD5) match the manifest's sizes and the stated MD5s. Last-Modified is 22 August 2025. ReShare's record states CC BY 4.0, and the attribution and DOI are recorded. A ReShare search found no deposit in the series newer than 856542. The registration is raw-only and emits no facts, so the consumer_fact.v5 conventions don't apply.
  • Part counts in the docs: ceil(3,214,161,126 / 100 MiB) = 31 and ceil(4,426,805,029 / 100 MiB) = 43.

CI at e120b65: Chronicle bundle and Chronicle surface were still running at 15:34 UTC.

The upload path looks sound. Item 1 is a time cost, not a correctness problem, and could go in this PR or a follow-up.

🤖 Generated with Claude Code

… MiB boundary

Review on #426 (Vahid, round 1):

- The S3 path now sends HEAD for the key first. The key carries the file's
  sha256, so an object of the file's size there already holds these bytes,
  and the file is not sent again. After a failed upload, such as a completion
  that committed but lost its response, the same check counts the upload once
  the object is there, and otherwise reports the upload's own error.
- A test puts sparse files of exactly 300 MiB and one byte more through
  _upload_r2_object: the first goes to Wrangler, the second to the S3 path.
- The manifest and doc note ReShare's temporal coverage, to 23 October 2023,
  beside the readme's and abstract's 31 October 2023.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juaristi22

Copy link
Copy Markdown
Collaborator Author

Thanks, Vahid. All three are in 555c440.

  1. Re-runs and lost completions. _upload_r2_object_in_parts now sends HEAD for the key before it uploads. When R2 already holds an object of the file's size there, the file is not sent again, and stdout reads already in R2. After a failed put_file, the same HEAD runs again:

    • if the object is there, the upload counts, and the error it reported stays in stdout;
    • if not, publish-raw reports the upload's own error, even when that second HEAD fails too.

    The check sits in artifacts.py, which knows that the keys carry the sha256. The client only gains R2Client.object_size, which returns None on a 404 and raises on any other refusal. The tests cover an object of the same size (not sent), one of another size (sent), and a completion that landed after a NoSuchUpload (counted). Against e120b65's artifacts.py, the same-size and landed-completion tests fail.

  2. The boundary. test_the_s3_path_starts_one_byte_above_the_wrangler_limit puts sparse files of exactly 314,572,800 bytes and one byte more through _upload_r2_object, with the limit unpatched. The first goes to Wrangler and the second to the S3 path. The test also pins the constant at 300 MiB.

  3. The end dates. The manifest's provenance notes and the doc now give ReShare's temporal coverage, 1 January 1995 to 23 October 2023, next to the 31 October 2023 that the readme and the record's own abstract give.

The four test files give 114 passed.

@juaristi22
juaristi22 marked this pull request as ready for review November 9, 2026 11:41
@vahid-ahmadi

Copy link
Copy Markdown

Round 2 at 555c440. I read the diff from e120b65 and your reply, ran the PR's three test files in a fresh clone with no Cloudflare credentials set (77 passed), ran the new tests against e120b65's artifacts.py, and re-ran the lost-completion probe against the real R2Client with a mocked transport. Nothing was sent anywhere.

Earlier items

  1. Fixed. _upload_r2_object_in_parts sends HEAD first and skips on a size match (chronicle/artifacts.py:1562-1565), and checks again after a failed put_file (:1570-1582). In the probe, the first CompleteMultipartUpload committed and timed out, the retry got 404 NoSuchUpload, the upload was aborted, and the second HEAD found the object: publish-raw reported a valid entry and recorded the R2 key. With the object already there, a re-run sent one HEAD and nothing else. Against e120b65's artifacts.py, the same-size and landed-completion tests fail (tests/test_chronicle_artifacts.py:798, :820), as you said.
  2. Fixed. tests/test_chronicle_artifacts.py:846 sends sparse files of 314,572,800 bytes and one byte more through _upload_r2_object with the limit unpatched, and pins the constant.
  3. Fixed. Both dates now sit side by side (db/data/ucl/house_price_per_square_metre_856542/manifest.yaml:58-59, docs/data-sources/uk-land-value-inputs-408.md:413-414).

Should

  1. The size-match shortcut also applies to publish-derived, whose keys carry no sha256. The docstring rests the skip on "the key carries the file's sha256" (chronicle/artifacts.py:1549). That holds for fetch-artifact (:558) and publish-raw (:1727), which use build_r2_key. publish-derived (:699) uses build_derived_r2_key (:1356-1383), which gives derived/{country}/{source_id}/{package_id}/{year}/{build_id}/{artifact_name}. In a probe, I put a 23-byte object under derived/uk/ons/pkg/2024/chronicle.build.v2:abc123/ledger.db and published a different 23-byte ledger.db with the same build ID. It sent one HEAD, returned already in R2, and the entry recorded the local file's sha256, which R2 does not hold. The check after a failed upload has the same gap: a failed upload of new bytes counts as landed if an older object of the same size is there. The trigger is narrow: a derived file above 300 MiB, published again under the same build ID, with the same size and different bytes. _build_id hashes the facts, cells and rows (chronicle/database.py:1000), not the files' bytes, and --build-id accepts any digest (chronicle/harness.py:1080). I didn't check whether a rebuilt ledger.db is byte-identical. Below 300 MiB, Wrangler overwrites the same key, so the two paths now behave differently. Fix: apply the skip and the post-failure check only where the key is content-addressed, for example a flag passed from the raw and fetch callers. Correct the docstring to match, and add one derived test.

Checked and fine

  • object_size returns None only on a 404 (chronicle/r2_s3.py:243-251, :345), and a 404 is not retried. A 403 on the first HEAD fails at once with r2_upload_failed and sends nothing. When the second HEAD fails, the upload's own error is reported (tests/test_chronicle_artifacts.py:771). object_size has tests at tests/test_chronicle_r2_s3.py:265 and :278.
  • For raw and fetched files, a size-only match is enough, since the same key means the same sha256.
  • As before, I didn't check the readme's definition of the measure inside bundle 1.

CI at 555c440: Chronicle bundle, Chronicle checks and Chronicle surface all pass.

Items 1 to 3 are done. Item 4 is limited to derived files above 300 MiB, and a flag on the two content-addressed callers would close it.

🤖 Generated with Claude Code

Review on #426 (Vahid, round 2, item 4): publish-derived shares the S3 path,
but its keys name a build, not the bytes, so an object of a derived file's
size under its key may hold other bytes. The HEAD before uploading, and the
one after a failed upload, now run only when the caller marks the key as
content-addressed: fetch-artifact and publish-raw, whose keys carry the
file's sha256. Derived files are always sent, as Wrangler sends smaller ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juaristi22

Copy link
Copy Markdown
Collaborator Author

Thanks, Vahid. Item 4 is in 1c219c1.

  1. The skip now needs a content-addressed key. _upload_r2_object takes content_addressed, which defaults to False.

    • fetch-artifact and publish-raw pass True: their keys come from build_r2_key and carry the sha256 of the file they have just hashed.
    • publish-derived leaves it off, so neither HEAD runs. The file is always sent, as Wrangler sends files up to 300 MiB, and a failed upload stays failed whatever is under the key.

    The docstrings say so, and the changelog fragment now names raw and fetched files. test_a_derived_file_is_sent_whatever_is_under_its_key follows your probe. A 14-byte ledger.db is sent although an object of its size is already under its derived key, and a failed upload stays failed when such an object is there afterwards. Against 555c440's artifacts.py, the test fails.

The four test files give 115 passed.

@vahid-ahmadi

Copy link
Copy Markdown

Round 3 at 1c219c1. I read the diff from 555c440 and your reply, ran the PR's three test files in a fresh clone with no Cloudflare credentials set (78 passed), ran the new derived test against 555c440's artifacts.py (it fails, as you said), and re-ran the derived-key probe against the real R2Client with a mocked transport. Nothing was sent anywhere.

Earlier items

Items 1 to 3 were fixed in round 2, and the raw path still behaves as it did then (below).

  1. Fixed. publish-derived calls _upload_r2_object without content_addressed (chronicle/artifacts.py:700-704), so it defaults to False (:1524). The first HEAD is skipped (:1574-1576), and a failed upload re-raises without the second one (:1586-1587). fetch-artifact (:562) and publish-raw (:1748) pass True. In the probe, with a different 23-byte object already under derived/uk/ons/pkg/2024/chronicle.build.v2:abc123/ledger.db, publishing the new 23-byte ledger.db sent no HEAD, did a full multipart upload, and R2 then held the new bytes. With the completion failing and the old object still there, the entry was invalid and reported the upload's error. On the raw path nothing changed: one HEAD and already in R2 when the object is there, and a lost completion still counts once the second HEAD finds it. The docstrings (:1526-1530, :1558-1565) and the changelog fragment now match, and tests/test_chronicle_artifacts.py:846 covers both derived cases.

No new findings. Still open from round 1: I didn't check the readme's definition of the measure inside bundle 1, and #375's government-only rule is left with Max.

CI at 1c219c1: Chronicle bundle, Chronicle checks and Chronicle surface all pass.

All four items are done.

🤖 Generated with Claude Code

@juaristi22

Copy link
Copy Markdown
Collaborator Author

Thanks, Vahid. On the open point: UCL's readme in bundle 1 (Readme .docx) doesn't define the measure itself. It names priceper, with classt and id, as the fields UCL made, and refers to the London Datastore page for what they mean. It says four fields but names three. Of the rest, the upper-case fields come from the EPCs, lad21cd from the NSPL and the others from Price Paid. What the manifest and the doc take from the readme matches it: 21,438,854 transactions from 1 January 1995 to 31 October 2023, with 107 fields.

#375 stays with Max. If it lands first, ucl needs a fourth exception, as the body says.

@juaristi22
juaristi22 merged commit 8f409d6 into main Nov 9, 2026
3 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