Complete the filesystem parity checklist: pipe_file, version-aware mode, and fsspec registry DX#730
Merged
Merged
Conversation
The inherited pipe_file went through the open() + write() buffered-file path even though the whole payload is already in memory. Override it to issue a single PutObject request for data up to the block size, and a parallel multipart upload (parts sized to fit within the 10,000-part quota, aborted on failure) for larger data. Supports the fsspec "create" mode and passes additional kwargs (e.g., ContentType) to the PutObject / CreateMultipartUpload APIs. AioS3FileSystem._pipe_file already delegates to the sync implementation and inherits the behavior. Part of the filesystem parity checklist in GH-722. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Preserve fsspec transaction semantics by deferring to the buffered open() path inside a transaction - Apply the filesystem-level s3_additional_kwargs and accept the open()-style block_size / max_worker / s3_additional_kwargs kwargs - Enforce the minimum/maximum part size on the multipart path and cap the single-PutObject branch at the maximum part size - Extract _finish_multipart_upload, shared with _copy_object_with_multipart_upload, which now also cancels remaining parts and aborts the upload on failure without masking the original error (abort failures are logged) - Split parts with a plain offset loop and size the executor by the part count; accept bytes-like values - Fix S3File._get_ranges generating an empty trailing range when the size is an exact multiple of the block size (latent read-path bug) - Add mode="overwrite" to AioS3FileSystem._pipe_file to match the fsspec AsyncFileSystem signature Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pyathena.pandas / pyathena.polars unconditionally clobbered the fsspec "s3" / "s3a" protocols at import time, silently shadowing s3fs (GH-719). The registration is still required for the pandas/polars result sets, but it now goes through pyathena.filesystem.register_s3_filesystem, which leaves a filesystem class that the user has already registered explicitly (present in fsspec.registry) untouched, logs what it does, and documents how to restore s3fs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- S3FileSystem/AioS3FileSystem accept version_aware (default False): reads pin the object version observed at open/head time, and ls(versions=True) lists all versions of a prefix - object_version_info lists the versions of the objects under a path as typed S3ObjectVersion instances (paginated, optional delete markers) - Add the s3:GetObjectVersion / s3:ListBucketVersions permissions to the GitHub Actions OIDC role for the integration tests Part of the filesystem parity checklist in GH-722. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
12 tasks
…y overwrite - Version-aware opens bypass the dircache so the pinned version is the one actually observed at open time, not a stale or version-less cached entry - ls(versions=True) on an object path falls back to listing the versions of the key itself instead of returning an empty list - Guard the ListObjectVersions pagination against responses without a NextVersionIdMarker - register_s3_filesystem now always registers PyAthena's filesystem class (live fsspec registry), logging a warning when it overwrites an explicitly registered implementation - Use f-strings for log messages in the filesystem modules and ignore ruff's G004 accordingly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Extract _list_object_versions_pages, shared by ls(versions=True) and object_version_info, and _directory_object/_versioned_file_object factories replacing the repeated S3Object literals - Move the version-aware staleness decision into info(): a version-less cached entry is re-headed there, removing the file-layer refresh workaround and the doubled HEAD on version-aware opens - Route S3File.commit's multipart branch through _finish_multipart_upload so the complete-or-abort protocol has a single owner (commit now also aborts on failure) - Bound the ls(versions=True) object-path fallback listing with a delimiter; drop the S3ObjectVersion round-trip in the fallback - Drop the dead clobber parameter of register_s3_filesystem and reuse the helper in the test fixture; remove mocked tests duplicating integration coverage - Add the docs/filesystem.md user documentation page and fix the RST list formatting in the S3FileSystem class docstring Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keep the test file layout mirroring the source layout instead of adding a separate test_registry.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pipe_file now writes only the single-PutObject case directly (the checklist item in GH-722) and defers larger data to the buffered open() path, so the multipart upload orchestration has a single owner in S3File instead of a second copy in pipe_file. The pipe-specific multipart tests are replaced by unit tests of the shared _finish_multipart_upload helper, which remains in use by the copy path and S3File.commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fsspec's AbstractFileSystem.mv() passed the typo'd "onerror" keyword (instead of "on_error", which copy() consumes) until it was fixed in fsspec 2026.6.0, so it leaks through copy(**kwargs) into cp_file on older releases and must not reach the S3 API. Replace the stale TODO with the actual mechanism and the fsspec version at which the pop can be removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Import Connection at the top of the module; the runtime import guarded against an import cycle that no longer exists - Replace the bare s3fs source link with a docstring describing the accepted arguments - Honor use_ssl=False, which the previous truthiness check silently dropped, and stop injecting None credential values into the session and client arguments Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
laughingman7743
added a commit
that referenced
this pull request
Jul 6, 2026
Convert the %-style logger calls outside the filesystem modules (which were converted in GH-730) to f-strings, unifying the logging style across the codebase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
WHAT
Completes the remaining items of the filesystem parity checklist in #722, designed around PyAthena's architecture (typed model classes, sync boto3 client with executor-based parallelism):
Direct-write
pipe_fileopen()path, so the multipart upload orchestration keeps a single owner (S3File) and transactions keep their deferred-commit semantics.s3_additional_kwargs, theopen()-styleblock_size/max_workerkwargs, and bytes-like values. Supports fsspec'smode="create"._finish_multipart_upload— the complete-or-abort protocol shared byS3File.commitand_copy_object_with_multipart_upload, which now cancel remaining parts and abort the upload on failure instead of leaking incomplete uploads (abort failures are logged without masking the original error).S3File._get_rangesbug that generated an empty trailing range when the size is an exact multiple of the block size (affected the concurrent read path too), and addsmodetoAioS3FileSystem._pipe_fileto match the fsspec signature.Version-aware mode and object version listing
version_aware=Falseconstructor flag on both filesystems: when enabled, reads pin the object version observed at open/head time, andls(path, versions=True)lists all versions of a prefix.object_version_info(path, delete_markers=False)lists the versions of the objects under a path as typedS3ObjectVersioninstances (paginated ListObjectVersions).s3:GetObjectVersion/s3:ListBucketVersionsto the CI OIDC role (CloudFormation stack deployed).fsspec registry DX (raised in #719)
pyathena.filesystem.register_s3_filesystem, which registers PyAthena's filesystem class in the live fsspec registry and logs a warning when it overwrites an explicitly registered implementation (previously the shadowing was completely silent — pyathena s3 fsspec replacement file system not writing when used with transactions & a context manager #719). The docstring documents how to restore s3fs by re-registering it afterwards.Async streaming reads (verification item)
Verified rather than changed:
AioS3Filedispatches parallel range reads throughS3AioExecutoron the event loop and is covered by the existing async read tests. fsspec'sopen_async/AbstractAsyncStreamedFileprotocol is intentionally not implemented — the executor-based model is the supported async read path.Testing (following the minimize-mocking policy): real-S3 integration tests for the pipe round-trips (incl. a 6 MiB multipart case),
mode="create"+ ContentType, transaction commit/rollback, object version listing (an unversioned bucket reports thenullversion), and version-aware reads; mocked tests only for request shapes and failure injection (part splitting, abort-on-failure, error masking, pagination); pure-logic unit tests forS3ObjectVersionand the registry helper.just lintpasses; changed-area tests pass against real AWS.WHY
Closes #722. Together with #729 (directory operations, metadata/tags/ACL, multipart-upload management, error translation,
S3Filehelpers), this completes the umbrella issue: users migrating from s3fs get the remaining API surface, and the registry no longer silently shadows an explicitly chosen implementation. The optional bucket-versioning helpers (make_bucket_versioned, etc.) are intentionally not implemented, consistent with the bucket-lifecycle design decision in #729 (infrastructure-level changes are out of scope for the filesystem; versioning configuration belongs to infrastructure management).🤖 Generated with Claude Code