[bulk] Tell an empty source apart from one load could not read (closes #340) - #341
Merged
Merged
Conversation
…#340) Zero rows meant one thing to the code and two things in practice. An empty drop is a legitimate input; a source that holds bytes and yields nothing is the reader failing to understand it. Both reported success, so a mistyped --format was indistinguishable from an empty file -- and Spark makes that worse by having its JSON reader return no rows where its Parquet reader raises, so the same operator error failed cleanly in one format and passed silently in another. Now: no bytes -> "Read 0 items from '<path>': the source is empty, so nothing was loaded." exit 0, as before bytes, 0 rows -> BulkExecutorError: "Read 0 items from '<path>', but it holds 29 bytes. The data is probably not 'json' -- check --format. (A header-only CSV also reads as 0 items.)" The byte count comes from a new s3_source_bytes(), called only in the zero-row branch, so the happy path pays nothing. It reuses the head_object/list_objects_v2 shape the existing existence check uses, with the URI split extracted so both share it, and it deliberately does not swallow a non-404 error -- reporting a denial as "the source is empty" would be worse than the bug this fixes. The header-only CSV false positive is named in the message rather than special-cased. Verified live: 29-byte non-JSON file fails with that sentence in 26 lines; a 0-byte file loads nothing and succeeds. The first live run also caught the message being wrapped twice ("Could not read the source ... as 'json': Read 0 items ..."), because the raise happens inside the read handler's try -- that handler now re-raises a BulkExecutorError untouched, with a test that fails if the passthrough is removed.
hunterhacker
changed the base branch from
main
to
driver-side-failures-reported-politely
September 1, 2026 23:06
hunterhacker
changed the base branch from
driver-side-failures-reported-politely
to
main
September 1, 2026 23:07
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
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.
Closes #340. Stacked on #339 — both edit the same function in
load/__init__.py, so merge #339 first; this branch contains it.The A/C split discussed on the issue: #339 stopped the contradiction (an
ERRORline above "Job completed successfully"); this is the behaviour change underneath.The problem
Zero rows meant one thing to the code and two things in practice:
Read 0 items from '<path>': the source is empty, so nothing was loaded.Read 0 items from '<path>', but it holds 29 bytes. The data is probably not 'json' -- check --format. (A header-only CSV also reads as 0 items.)Spark makes the old behaviour worse than it sounds: its JSON reader returns no rows where its Parquet reader raises, so
--format parquetat a CSV file already failed cleanly (Could not read the source at '...' as 'parquet') while--format jsonat the same file passed silently. Same operator error, opposite outcomes.How the byte count is obtained
A new
s3_source_bytes(), called only in the zero-row branch, so the happy path pays nothing for it. It reuses thehead_object→list_objects_v2shape the existing existence check already uses (single object costs one call; a prefix pages and sums), with the URI split extracted into_split_s3_uriso both share it.It deliberately does not swallow a non-404
ClientError: reporting a denial as "the source is empty" would be a worse bug than the one being fixed. There is a test for that.The header-only CSV
A CSV with just a header has bytes and zero data rows, so it now fails. Per the issue discussion I took option 1 — name it in the message rather than special-case it, since a header-only file is usually a mistake too and detecting it needs a second read.
Verified live
--format jsonBulkExecutorError: Read 0 items from 's3://…/data.json', but it holds 29 bytes. The data is probably not 'json' -- check --format.--format jsonWARNING - Read 0 items from 's3://…/data.json': the source is empty, so nothing was loaded.thenJob completed successfullyCaptures in
~/Documents/bulk-340-runs/.The first live run caught a bug the unit tests had not: the message came out wrapped twice —
Could not read the source ... as 'json': Read 0 items ..., but it holds 29 bytes ...— because the zero-row raise happens inside the try whose handler converts read failures. That handler now re-raises aBulkExecutorErroruntouched, and a test fails if the passthrough is removed.Tests
make test: 1690 passed, 48 skipped.load/__init__.pyat 99%.Mutation-checked: reverting the warning to
log.error, removing theBulkExecutorErrorpassthrough (double-wrap returns), and five tests fors3_source_bytescovering single object, paged prefix, empty prefix, a non-404 error propagating, and a malformed URI.