Settle the response models, S3Object, and the listing cache #1341

Activity

1. laughingman7743

   added

   enhancement

   on Oct 24, 2026
2. laughingman7743

   added a parent issue on Oct 24, 2026

   • Audit the structure and implementation of the filesystem package #1327
3. laughingman7743 commented on Nov 7, 2026

   

   laughingman7743

   on Nov 7, 2026

   MemberAuthor

   More actions

   Deferred from #1370 (recursive chmod, GH-1335), pre-existing, found by its independent review; they concern how paths and listed names are identified, so they belong with the response model here:

   1. Listed names reparsed as version paths. With the literal keys a/x and a/x?versionId=null (no versioning), find("bucket/a") names the second bucket/a/x?versionId=null; chmod(recursive=True) and rm(recursive=True) reparse that name as the null version of a/x, so a/x is targeted twice and the literal key not at all.
   2. bucket// is both the bucket and the key /. S3Path.parse("bucket//") keeps the key /, while find() strips it to the bucket. chmod("s3://bucket//", acl, recursive=True) updates the objects but skips PutBucketAcl (as on master).
   3. PathLike paths. chmod(Path("bucket/a"), ...) raises TypeError because S3Path.parse receives the Path before _strip_protocol converts it.
4. laughingman7743 commented on Nov 7, 2026

   

   laughingman7743

   on Nov 7, 2026

   MemberAuthor

   More actions

   Design audit at 29cc1d3 (offline: stub client, no AWS), before deciding the proposal above. It re-checks the issue text and the three deferred items of the comment above, inventories the callers, and records the decisions. Codex reviewed a first draft; its factual corrections were reproduced and are folded in.

   Decided (2026-11-07): the recommended option of each item below. The sub-issues follow in the next comment.

   Current state (29cc1d3)

   Reproduced with a stub client unless marked "read".

   1. Models.
      • Two styles in two modules: frozen dataclasses with from_response in s3_core.py, property classes over raw dicts in s3_object.py. The import cycle exists only under TYPE_CHECKING (s3_object.py:14).
      • S3ObjectVersion("b", False, {"Key": "k", "VersionId": "v1"}).name is b/k.
      • Checksum properties: 10 on S3MultipartUploadPart, 4 on S3PutObject and S3CompleteMultipartUpload, none on S3Metadata. The installed botocore defines the same 10 algorithms (plus ChecksumType, except UploadPart) as optional output fields of PutObject, CompleteMultipartUpload, HeadObject, GetObject and the CopyObjectResult of CopyObject.
   2. S3Object.
      • Not a dict: isinstance(entry, dict) is False, repr() is the default <S3Object object at ...>, json.dumps raises.
      • Defect 22: after entry["keys"] = 1, dict(entry) raises TypeError: 'int' object is not callable; after entry["get"] = 1, entry.get("size") raises the same.
      • entry.update({"ContentType": "x"}) stores ContentType, while __init__ maps it to content_type.
      • Directory entries hold content_type, etag, last_modified and version_id as None; file entries omit the fields that the response lacks.
      • Cost on this machine: S3Object.from_summary 2.36 µs and copy() 1.20 µs; a prototype dict subclass 0.23 µs and its dict.copy() 0.07 µs (not a type-preserving copy).
      • Correction to the proposal: a dict subclass is JSON-serializable only for directory entries; file entries hold last_modified as a datetime, as s3fs's entries do.
   3. S3Metadata. bool(S3Metadata({"ContentLength": 5})) is False, and two objects without user metadata compare equal to each other and to {}. This is the documented mapping behavior of fs.metadata(); no caller in the package tests its truth value (read).
   4. S3Core parameters and results (read).
      • Four precedence rules: sent as given, with TypeError on a duplicate (head_object, copy_object, delete_object, tagging, ACL, listings); the method's fields win (get_object, put_object, the multipart methods, list_object_annotations); params win over retained fields but TypeError for the fields the copy sets (replace_object_metadata); params win over the path (generate_presigned_url). call() adds request_kwargs under the request's own parameters (s3_core.py:651); Send the authorization parameters of s3_additional_kwargs with every filesystem request #1372 extends which filesystem parameters reach it.
      • copy_object, delete_object and replace_object_metadata return None.
      • to_dict() of S3PutObject and S3CompleteMultipartUpload returns the private attribute names (_etag, ...), and touch() returns it (s3.py:1491; asserted in test_s3.py:1276). s3fs's touch returns the raw PutObject response.
   5. S3Path.
      • Access point ARNs do not parse (ValueError); s3fs 2026.9.0 splits arn:aws:s3:us-east-1:123456789012:accesspoint/ap/k into the ARN and k.
      • S3Path("b", "") prints b and re-parses with key None.
      • Defect 23 holds. s3fs has a related limit: split_path("bucket/data?versionId=1") drops the query and gives the key data.
   6. Listing cache.
      • Keys: path (HeadObject/HeadBucket), "" (buckets), (path, "/") and (path, "") (listings), (path, "lookups") (lookups with parameters). _invalidate_path (s3.py:2586) evicts four keys for each of the four query spellings of each ancestor.
      • info() and ls(detail=True) return cached entries by reference: after fs.info("bucket/a")["size"] = 999 the next info() returns 999; the same for ls. fsspec's find/walk/du do not mutate entries (read); s3fs's _lsdir also returns its cached list.
      • AioS3FileSystem shares the sync dircache (s3_async.py:161), and its _find calls the sync _find (s3_async.py:788).

   Deferred items of the comment above:

   • C1 (listed names reparsed). With the literal keys a/x and a/x?versionId=null in an unversioned bucket, find("bucket/a") names bucket/a/x?versionId=null, and:
     • chmod(recursive=True) updates a/x twice (once with VersionId=null) and never the literal key;
     • rm(recursive=True) deletes a/x and a/x version null, and leaves the literal key;
     • copy(recursive=True) raises ValueError: Cannot copy to a versioned file. after copying a/x;
     • mv(recursive=True) raises ValueError: Cannot move several paths to the same destination. before copying;
     • du (fsspec's, through info() of each name) counts a/x once under one name;
     • info("bucket/a/x?versionId=null") looks up a/x with VersionId=null (defect 23).
     • Traversal and invalidation lose identity too: with the key a/dir?versionId=v1/z, find("bucket/a") lists it but find("bucket/a", maxdepth=2) does not, since _find_levels (s3.py:930) re-parses the directory name; _invalidate_path (s3.py:2594) can take part of such a key for a version and skip the parent's listing.
   • C2 (bucket//). S3Path.parse("b//") is the key / with is_bucket True. find("bucket//") and recursive chmod("s3://bucket//") list the bucket; the recursive chmod sends no PutBucketAcl. rm_file("bucket//") deletes the key /. So the spelling is the key / for single-object operations and the bucket for listings.
   • C3 (PathLike) with pathlib.Path("bucket/a"): chmod, cat_file, rm_file, touch, metadata, sign, getxattr, setxattr, get_tags, object_version_info, cp_file, list_multipart_uploads and put_file (remote path) raise TypeError from S3Path.parse; rm raises TypeError: 'PosixPath' object is not iterable; glob raises AttributeError (endswith). info, exists, ls, find, du, size and checksum work through fsspec's _strip_protocol.

   Callers and compatibility surface

   • S3Object is returned by info, ls/find/glob/walk with detail=True, the details of an open file and their aio counterparts, is stored in fs.dircache, and is in the API reference (docs/api/filesystem.rst:113). Its to_api_repr() feeds append mode (Let the caller's parameters take precedence in append mode #1373 changes that precedence), and created/modified and read-version/ETag pinning read its fields.
   • S3Core, S3Path, S3PutObject, S3MultipartUpload and the page models are documented in docs/filesystem.md; S3Metadata is the documented return of fs.metadata()/S3File.metadata() and the head argument of S3Core.replace_object_metadata, with a published {**head, ...} example (docs/filesystem.md:563).
   • Tests: 81 references to dircache under tests/pyathena/filesystem; Audit the filesystem tests changed since the #1239 audit #1328 left the design-dependent cache/model test rewrites to this issue.
   • Parallel work on the same code: Send the authorization parameters of s3_additional_kwargs with every filesystem request #1372/PR Send the authorization parameters of s3_additional_kwargs with every request #1377 (filesystem authorization parameters for every request), Let the caller's parameters take precedence in append mode #1373 (append precedence over to_api_repr()), Run the batch operations of S3FileSystem concurrently (GH-1322) #1344 (Run the batch operations of S3FileSystem concurrently #1322 step 4a: copy/mv pairing and the rm/chmod fan-outs), Remove the duplication between S3FileSystem and AioS3FileSystem #1340 (sync/aio duplication).

   Decisions

   1. S3Object as a dict subclass. Recommended: yes, with attribute access for known fields (None for an absent known field, AttributeError otherwise), attribute set/delete mapped to the entries, copy() returning an S3Object, to_dict()/to_api_repr() unchanged, and the S3Object(init, **kwargs) constructor still mapping API names. Entries keep their current key sets (directories keep their None-valued keys), so entry["etag"] does not change to KeyError. Breaking: isinstance(entry, dict) becomes True and the pickled form changes (release note).
   2. Cached entries. Recommended: every boundary that returns entries (info, ls/find/glob/walk details, file details, aio _find) returns copies that also copy the nested metadata mapping; the cost is measured on the type-preserving copy before merging.
   3. Response models, compatible part. Recommended: one module of models with one style, re-exported under the current names, constructors still accepting response dictionaries; checksum properties per operation as botocore defines them; S3ObjectVersion gains a typed path (its name stays bucket/key). Whether pickles of the property classes must still load is part of this decision; recommended: no (release note).
   4. HeadObject result. Recommended: keep S3Metadata for S3Core.head_object and fs.metadata() and document that its truth value and equality are those of the user metadata; no new type. A new record type would break the published {**head, ...} usage for little gain.
   5. Core results. Recommended: copy_object/replace_object_metadata return an S3CopyObject (destination version ID, source version ID, ETag, last modified, checksums) and delete_object an S3DeleteObject (version ID, delete marker), in place of None. touch() options: (a) unchanged, documented; (b) the raw PutObject response, as s3fs; (c) public field names. Recommended: (b) in the release with the other breaking changes, since (c) is breaking without gaining s3fs parity.
   6. One **params rule. Recommended principle: fields that a method derives from its arguments raise TypeError when also given in params; defaults the method fills in (retained metadata, request_kwargs) yield to params. The PR first tabulates each method's derived fields (including CopySourceRange, MultipartUpload, checksum and pagination fields), keeps documented fallbacks such as range_=None with Range=, and normalizes the writer's internal calls (s3_writer.py:219) that forward checksum parameters.
   7. Literal keys that look like version paths (defect 23, C1). Recommended: filesystem operations carry the listed entries' bucket, key and version ID through traversal (_find_levels), pairing, the fan-outs, lookups and invalidation, using a private sequence of typed paths; public find()/expand_path() still return strings. Addressing such a key by a string path stays a documented limit, as in s3fs, with S3Core and an explicit S3Path as the way to reach it; no escape syntax, since any spelling is itself a possible key.
   8. bucket// (C2). Options: (a) the bucket for every operation (removes rm_file's access to the key /); (b) keep the current split and document it, including that a recursive chmod of bucket// does not change the bucket ACL. Recommended: (b).
   9. PathLike (C3). Recommended: every public method converts os.PathLike with fsspec.utils.stringify_path before classifying the path (including rm's iterable check, glob and the pairing's isinstance(..., str)), without stripping slashes.
   10. S3Path details. Recommended: S3Path(bucket, "") normalizes the key to None. Access point ARNs as buckets become a separate enhancement issue, since parent, pairing and cache handling need ARN-aware buckets.
   11. Listing cache object. Recommended: one S3ListingCache over fs.dircache (still fsspec's DirCache, so use_listings_cache, listings_expiry_time, max_paths and empty-cache pickling keep working), shared by both filesystems, that owns the key scheme. It keeps today's behavior: the four version-query spellings, explicit version_id, trailing-slash and empty-segment variants, eviction of the bucket listing only when it lists the bucket, uncached null versions, no caching for prefix/token listings, per-parameter lookups with their own timestamps and hashed SSE-C keys, and the _ls_from_cache override. Tests that read dircache keys are rewritten in that PR, keeping their request/expiry/invalidation evidence.

   Split

   One sub-issue of this issue per PR:

   • A1. PathLike (item 9). Small and independent; first.
   • A2. Path identity (items 7, 8, key="" of 10). After Run the batch operations of S3FileSystem concurrently (GH-1322) #1344 merges, since both rewrite pairing and the fan-outs; by the owner of Run the batch operations of S3FileSystem concurrently (GH-1322) #1344.
   • B. Entries (items 1–2).
   • C. Models and core results (items 3–6). The compatible consolidation first; the breaking changes (touch(), the **params rule) in the same release.
   • D. Cache object (item 11). Its interface settled before Remove the duplication between S3FileSystem and AioS3FileSystem #1340; independent of B.
   • ARN support: a separate issue.
5. laughingman7743

   added sub-issues on Nov 7, 2026

   • Accept os.PathLike paths in every S3 filesystem method #1380
   • Keep the identity of listed keys through traversal, fan-outs, and invalidation #1381
   • Make S3Object a dict subclass and return copies of cached entries #1382
   • Consolidate the S3 response models and settle the core results and parameter precedence #1383
   • Own the listing cache key scheme and invalidation in one cache object #1384
6. laughingman7743 commented on Nov 7, 2026

   

   laughingman7743

   on Nov 7, 2026

   MemberAuthor

   More actions

   Sub-issues of the decisions above, one PR each:

   1. Accept os.PathLike paths in every S3 filesystem method #1380 — os.PathLike paths in every method (decision 9). First; small and independent.
   2. Keep the identity of listed keys through traversal, fan-outs, and invalidation #1381 — identity of listed keys through traversal, fan-outs and invalidation; bucket// documented; key="" normalized (decisions 7, 8, 10). After Run the batch operations of S3FileSystem concurrently (GH-1322) #1344 merges.
   3. Make S3Object a dict subclass and return copies of cached entries #1382 — S3Object as a dict subclass and copies of cached entries (decisions 1, 2).
   4. Consolidate the S3 response models and settle the core results and parameter precedence #1383 — response models, core results, touch() and the **params rule (decisions 3–6).
   5. Own the listing cache key scheme and invalidation in one cache object #1384 — one listing cache object (decision 11). Interface settled before Remove the duplication between S3FileSystem and AioS3FileSystem #1340.

   Access point ARNs (decision 10) are split out to #1385.