Skip to content

fix: stream file copies to allow outputs over 5GB TDE-2072 - #1653

Open
schmidtnz wants to merge 14 commits into
masterfrom
fix/multipart-upload-tde-2072
Open

schmidtnz wants to merge 14 commits into
masterfrom
fix/multipart-upload-tde-2072

Conversation

@schmidtnz

@schmidtnz schmidtnz commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Motivation

16-bit COGs may exceed 5GB, which PutObject rejects with EntityTooLarge.

Modifications

fs.copy() streams instead of buffering, using upload_file for s3 so multipart uploads are handled automatically. It now returns the multihash rather than the target path. Call sites doing write(target, read(source)) moved to it.

STAC reuses the multihash instead of reading the asset back to checksum it.

Verification

Unit tests for the streaming helpers, all four copy() directions, and the checksum passthrough; file:checksum values pinned against precomputed hashes.

IN PROGRESS: Mackenzie RGBNIR workflow test run to confirm the >5GB upload end to end.

@schmidtnz schmidtnz added container Publish a container and removed container Publish a container labels Sep 16, 2026
@schmidtnz
schmidtnz marked this pull request as ready for review September 16, 2026 23:57
@schmidtnz
schmidtnz requested a review from a team as a code owner September 16, 2026 23:57
return file_multihash
except FileNotFoundError as error:
raise NoSuchFileError(source) from error
except client("s3").exceptions.ClientError as ce:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could the exception be handled without creating another S3 client here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed (also in pre-existing places)

ClientError
"""
start_time = time_in_ms()
os.makedirs(os.path.dirname(destination), mode=0o777, exist_ok=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is destination always expected to have a directory? Otherwise, could os.path.dirname(destination) return an empty string and cause os.makedirs() to fail?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@schmidtnz
schmidtnz requested a review from ctali September 17, 2026 04:52
ClientError
"""
start_time = time_in_ms()
with fs_local.atomic_write_path(destination) as partial_destination:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this leave an empty file behind if _get_object_body() fails?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

container Publish a container

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants