feat(ingestion): add the shared backfill primitives - #849
Merged
Conversation
Chunking, run keys, location validation, and checkpoint tracking for both backfill modes. All pure, so the logic is testable without object storage or a database. Written from this plan's description rather than ported line by line: the Aqueduct checkout available here is an empty directory skeleton. Each docstring records provenance and what differs so the two can still be diffed. ChunkResult counts rows_upserted rather than Aqueduct's posted/deleted pair. That is not a rename. Aqueduct deletes a window and re-posts it because FROST has no constraint to conflict on, which gives it two numbers and a window where the data is missing; with the constraint from 3.4 Ocotillo upserts, so there is one number and no window. Three behaviours are deliberate and tested. Chunk edges clip rather than widen, so a backfill does not fetch data nobody asked for. An empty or reversed window is rejected rather than treated as a no-op, because a run that succeeds having done nothing looks exactly like one that worked. An unknown location id fails the run naming every bad id, for the same reason. Run keys are sanitized before becoming path segments: one containing a slash would write checkpoints somewhere a resumed run would not look for them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.49% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
|
Your pull request is automatically being deployed to Dagster Cloud.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task 4.1.
automated_ingestion/shared/backfill.py, 27 tests, all pure — nodatabase, no network.
Written from the description, not ported
The plan says "port from Aqueduct". The Aqueduct checkout available here is an
empty directory skeleton — no source — so these were rebuilt from this plan's
description of them. Each docstring records provenance and what differs, so the
two can still be diffed by someone with both repositories open.
Flagging it because "ported" implies line-level fidelity that I cannot claim.
rows_upsertedreplaces posted/deletedNot a rename. Aqueduct deletes a window and re-posts it because FROST has no
constraint to conflict on — hence two numbers, and a window during which the
data is missing. With the constraint from #847 Ocotillo upserts: one number, no
window.
Behaviours that are deliberate
location_idfails the run, naming every bad idMarking a chunk under
march gapand resuming undermarch-gapfinds the samecheckpoint — there is a test, because that asymmetry would otherwise silently
redo work.
Deferred
atomic_write_json_with_retry()and a GCS-backed checkpoint store wait until4.2 needs persistence. The
CheckpointStoreinterface and an in-memoryimplementation are here now, which is what makes the chunking and resumption
logic testable without object storage.
Not yet buildable
4.2 and 4.3 need point → deployment resolution, which is 3.2 — still blocked on
the 38-vs-33 monitoring point discrepancy. This is the part of T4 that does not
depend on it.
🤖 Generated with Claude Code