Skip to content

Complete the filesystem parity checklist: pipe_file, version-aware mode, and fsspec registry DX#730

Merged
laughingman7743 merged 12 commits into
masterfrom
feature/pipe-file-direct-put-722
Jul 6, 2026
Merged

Complete the filesystem parity checklist: pipe_file, version-aware mode, and fsspec registry DX#730
laughingman7743 merged 12 commits into
masterfrom
feature/pipe-file-direct-put-722

Conversation

@laughingman7743

@laughingman7743 laughingman7743 commented Jul 5, 2026

Copy link
Copy Markdown
Member

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_file

  • Data up to the block size is written with a single PutObject request (the checklist item); larger data and writes inside an fsspec transaction go through the buffered open() path, so the multipart upload orchestration keeps a single owner (S3File) and transactions keep their deferred-commit semantics.
  • Preserves the previous behaviors of the inherited path: the filesystem-level and per-call s3_additional_kwargs, the open()-style block_size / max_worker kwargs, and bytes-like values. Supports fsspec's mode="create".
  • Extracts _finish_multipart_upload — the complete-or-abort protocol shared by S3File.commit and _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).
  • Fixes a latent S3File._get_ranges bug that generated an empty trailing range when the size is an exact multiple of the block size (affected the concurrent read path too), and adds mode to AioS3FileSystem._pipe_file to match the fsspec signature.

Version-aware mode and object version listing

  • version_aware=False constructor flag on both filesystems: when enabled, reads pin the object version observed at open/head time, and ls(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 typed S3ObjectVersion instances (paginated ListObjectVersions).
  • Adds s3:GetObjectVersion / s3:ListBucketVersions to the CI OIDC role (CloudFormation stack deployed).

fsspec registry DX (raised in #719)

Async streaming reads (verification item)

Verified rather than changed: AioS3File dispatches parallel range reads through S3AioExecutor on the event loop and is covered by the existing async read tests. fsspec's open_async / AbstractAsyncStreamedFile protocol 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 the null version), 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 for S3ObjectVersion and the registry helper. just lint passes; changed-area tests pass against real AWS.

WHY

Closes #722. Together with #729 (directory operations, metadata/tags/ACL, multipart-upload management, error translation, S3File helpers), 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

laughingman7743 and others added 4 commits July 5, 2026 22:33
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>
@laughingman7743 laughingman7743 changed the title Write pipe_file with direct S3 API calls Complete the filesystem parity checklist: pipe_file, version-aware mode, and fsspec registry DX Jul 5, 2026
laughingman7743 and others added 8 commits July 5, 2026 23:36
…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 laughingman7743 marked this pull request as ready for review July 6, 2026 01:27
@laughingman7743 laughingman7743 merged commit 7ff6cdd into master Jul 6, 2026
17 checks passed
@laughingman7743 laughingman7743 deleted the feature/pipe-file-direct-put-722 branch July 6, 2026 01:43
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>
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.

Improve s3fs / fsspec API parity for the built-in S3FileSystem

1 participant