Answer info() and exists() from cached parent listings and fix bucket lookups - #1006
Conversation
info() and exists() looked up only string dircache keys, so the listings that _ls_dirs() caches under (path, delimiter) never answered them, and a bucket missing from the cached bucket listing was reported missing without HeadBucket. Look up the (parent, "/") listing for key paths, preferring an object to a key prefix of the same name as HeadObject does, and fall through to HeadBucket for buckets that the listing of owned buckets does not contain. invalidate_cache() of the root now removes the cached bucket listing, and exists() treats a bucket that HeadBucket denies (403) as existing, so makedirs(exist_ok=True) works for a prefix in such a bucket. Closes #965, closes #980. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| if version_id: | ||
| return None | ||
| if key: | ||
| parent_cache = self.dircache.get((self._parent(path), "/")) |
There was a problem hiding this comment.
Self-review round 1 (implementation behavior) — base 9b2f033708b81dce7323dfda61e8b2ec95df23da, head 2efc5a10e90a8332d998ba6159a0a673b16c96a7.
Covered: _ls_from_cache(), info(), exists(), invalidate_cache(), the mkdir/rmdir comments, and all tests changed in tests/pyathena/filesystem/test_s3.py. Traced the info() callers that now get listing entries: S3File.__init__ (size, ETag for IfMatch, version pin under version_aware), cat_file() negative ranges (info.key != key guard), cp_file() size, checksum() ETag, modified(). Also checked AioS3FileSystem._info/_exists, which delegate to the sync methods. The cached listing keys come only from ls()/_find(), which pass protocol-stripped paths, so (self._parent(path), "/") matches them. Prefixed and continuation listings are not cached, so a cached parent listing is complete.
Write paths: pipe_file, put_file, touch, cp_file, setxattr, _delete_objects_request, S3File.commit and the aio _put_file/_cp_file all call invalidate_cache(path), which evicts (parent, "/") and (parent, "") up to the bucket. So writes through this instance never leave a parent listing that misses the new object.
Result: FINDINGS (1).
- Release-note completeness. Before this change, a stale listing affected only
ls()/find(). Now it also answersinfo()/exists()/isfile()/size()/open(), so an object that another writer adds after the listing was cached is reported missing untilrefresh=True/invalidate_cache(). This is the intended S3FileSystem.info() does not use cached directory listings #965 behavior and matches s3fs. Repair: added it to the PR body's behavior changes. No code change.
No other findings: the version-qualified guard, the version_aware re-head, the same-name preference (object over prefix) and the bucket fallback each have a test. The exists() 403 branch is covered offline only.
| if isinstance(caches, list): | ||
| cache = next((c for c in caches if c.name == path), None) | ||
| matches = [c for c in caches if c.name == path] | ||
| # A key can be both an object and a key prefix. |
There was a problem hiding this comment.
Self-review round 2 (claims, callers, operations) — base 9b2f033708b81dce7323dfda61e8b2ec95df23da, head 2efc5a10e90a8332d998ba6159a0a673b16c96a7.
Claims checked:
- "Tuple keys since v3.15.0":
git tag --contains 681e7496→ first tag is v3.15.0. ✔ - "isdir/isfile/size/du call info()": fsspec 2026.9.0
isdir/isfile/sizecallinfo().ducallsfind()andisdir(), and reachesinfo()only through_find's object fallback. Corrected the PR body to name onlyisdir()/isfile()/size(). - "s3fs behaves the same way" (stale parent listing): s3fs 2026.9.0
_infouses fsspec_ls_from_cache, which raisesFileNotFoundErrorfrom a parent listing without the path. ✔ s3fs also answers "directory" from a listing of the path itself, which this PR deliberately does not do. - "ListBuckets returns only owned buckets" (docstring/comment): AWS ListBuckets doc, "buckets owned by the authenticated sender". ✔
- HeadBucket 403/404 statements: measured live with boto3 on 2026-10-03. ✔ "With invalid credentials the next request fails with PermissionError": measured
ls/pipe/info→PermissionErrorafterexists()returned True. ✔ docs/filesystem.md:101-104("info/isfile/opentreatdir/asdir: the objectdirif it exists, otherwise the directory"): still holds with a cached parent listing, because the object entry is preferred (this line). It would not have held if the path's own listing were used. No other docs mention the listing cache orexists()of buckets. The 403 →PermissionErrortable stays true for the other operations.
Adversarial callers:
- A key with
//has a parent with a trailing slash, which never matches a stripped listing key, so it falls through to HeadObject (no falseFileNotFoundError). - A version-qualified path never uses listings.
- A folder marker
dir/is listed as a CommonPrefix of the parent, so it does not shadow the objectdir. - External writers after a cached listing are recorded as a release-note behavior change (round 1).
- The stale-write race is out of scope by decision.
Evidence: the live tests/pyathena/filesystem run (422 passed) was local at this head. Draft CI ran only check/lint/offline (all pass); AWS jobs run on Ready. The exists() 403 path is offline-only.
Result: FINDINGS (1, PR-text only, repaired). No code change.
A cached listing entry has no ContentType, Metadata or encryption settings, so an append that looked up the existing object through it dropped them from the rewritten object. Look the object up with HeadObject for an append. A refreshed info() or exists() of a listed key that S3 no longer has left the parent listing in place, so the next lookup answered from it again. A refreshed lookup of a key now removes the cached listing of its parent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| # The rewritten object keeps the metadata of the existing one, | ||
| # which a cached listing entry lacks, so look up the object. | ||
| with contextlib.suppress(FileNotFoundError): | ||
| append_info = fs.info(path, refresh=True) |
There was a problem hiding this comment.
Independent review (relayed): Codex CLI 0.160.0, model gpt-6-astra, session 01a10169-e169-7793-aa8c-9e5591e0c2a0. Static review only, run with codex exec -s read-only on a detached snapshot at head 2efc5a10e90a8332d998ba6159a0a673b16c96a7 (merge-base 9b2f033708b81dce7323dfda61e8b2ec95df23da). The prompt included the diff range and the intended behavior, without the PR number, description or self-review records. Afterwards the snapshot was clean and still at the same HEAD.
Covered: info()/exists(), listing cache keys, object/prefix precedence, refresh and invalidation, bucket lookup/403, root paths, trailing/repeated slashes, folder markers, version queries, S3File reads/appends, cat_file, cp_file, checksum, find, DirFileSystem, async delegation, docstrings and tests.
Result: FINDINGS
- P2, introduced: append loses metadata after a cached listing.
info()now returns a ListObjectsV2 entry, soS3Fileappend (s3_additional_kwargs.update(append_info.to_api_repr())) omitted ContentType/Metadata from the rewritten object. - P2, introduced: a refreshed missing key reappears from its parent cache. After
ls("bucket/d")and an external delete,exists(..., refresh=True)is False, but_head_objectevicts only the key entry, so the nextexists()returns True from(bucket/d, "/"). - P2, pre-existing:
info("")/info("/")/info("s3://")callparse_path("")before the root branch and raise ValueError (same on the merge-base).
The reviewer noted that the own-listing, version_aware and explicit-version tests also pass on the merge-base; they are compatibility guards.
Verification by the author: all three reproduced (3 also on master). Repairs for 1 and 2 are in 1b26dd1 (this line and the refresh eviction in info()). 3 is outside this PR's scope and is reported to the maintainer instead of being fixed here.
There was a problem hiding this comment.
Repair 1b26dd1 — self-review of the repair, both perspectives (range: git range-diff 9b2f0337..2efc5a10 9b2f0337..1b26dd13; only the new commit was added).
Round 1 (behavior):
- The append lookup is now
info(path, refresh=True)withFileNotFoundErrorsuppressed. A missing object gives a plain write, asexists()→ False did. A prefix-only path still gets the directory object and then fails incat(), as on master. Other errors (e.g.PermissionError) still propagate before the base initializer (test_open_append_lookup_failure, now stubbinginfo). A versioned append is rejected earlier (s3.py:2622). info(refresh=True)of a key without a version evicts(parent, "/")before the lookup. Buckets are untouched (_head_bucketalready evicts""on 404), and version lookups do not touch listings. The internal version_aware re-head setsrefreshafter this point, so it does not evict.- New tests
test_open_append_keeps_metadata_of_listed_objectandtest_refresh_drops_cached_parent_listingfail on 2efc5a1 and pass here.
Round 2 (claims/operations):
- The
info()docstring forrefreshand the append comment match the code. - Request count: an append sends one HeadObject, the same as master's uncached
exists()+ cachedinfo(). It sends one more only when a HeadObject result was already cached. Stated in the PR body. - An append that is discarded after the lookup has evicted the parent listing needlessly. The cost is one relist, and a committed append invalidates it anyway.
Validation at 1b26dd1: just lint passed; uv run --env-file .env pytest -q -n 4 tests/pyathena/filesystem 424 passed (live S3).
Result: CLEAN. An independent follow-up on the repair is requested next.
There was a problem hiding this comment.
Independent follow-up (relayed): Codex CLI 0.160.0, model gpt-6-astra, session 01a10176-1537-7db2-96ed-5340a7bf6509. Static, read-only review of repair 1b26dd13991f98d42d91a0f72ea89c1654eae10d (git range-diff 9b2f0337..2efc5a10 9b2f0337..1b26dd13). Afterwards the snapshot was clean and still at the same HEAD.
Result: FINDINGS. Both original regressions are fixed and both new tests fail on 2efc5a1. One P2 remains, introduced relative to the merge-base: the parent-listing eviction applied only to info(refresh=True). ls("bucket/d/key", refresh=True) of a deleted object (which calls _head_object() directly) and the version_aware re-head both left ("bucket/d", "/") in place, so a later exists() returned True.
Repair fed2d2a0ec728c4539d38a556635e4cbeb6e7e6c moves the eviction to where the contradiction is found:
_head_object()404 without a version evicts(parent, "/")only if that listing still lists the path (mirrors_head_bucket()with""). Sols()of a nonexistent prefix keeps a correct parent listing.info()'s ListObjectsV2 check evicts the parent listing when it finds a key prefix, because nothing caches the prefix and the listing may predate it (a refreshed lookup would otherwise be followed byFileNotFoundErrorfrom the stale listing).- The
info(refresh=True)eviction from 1b26dd1 is removed.
Self-review of the repair:
- Round 1: every key-path
_head_object()caller (ls()ats3.py:563,info()) gets the eviction. Version-qualified misses keep listings. The non-refreshinfo()path reaches HeadObject with a listing cached only for the version_aware re-head, and the prefix branch only after a HeadObject miss. - Round 2: the
_head_object()/info()docstrings and the PR body now describe the actual eviction points.
Tests: test_missing_object_drops_cached_parent_listing[exists|ls|version_aware] and test_refreshed_prefix_drops_cached_parent_listing fail on 2efc5a1. [ls] and [version_aware] also fail on 1b26dd1. test_missing_object_keeps_cached_parent_listing_without_it guards the conditional eviction. just lint passed; live tests/pyathena/filesystem 428 passed at fed2d2a. A second independent follow-up is requested next.
There was a problem hiding this comment.
Second independent follow-up (relayed): Codex CLI 0.160.0, model gpt-6-astra, session 01a1017d-e4f4-7672-b57f-e66cdce4d0c3. Static, read-only review of repair fed2d2a0ec728c4539d38a556635e4cbeb6e7e6c (git range-diff 9b2f0337..1b26dd13 9b2f0337..fed2d2a0). Afterwards the snapshot was clean and still at the same HEAD.
Covered: _head_object(), cache helpers, ls(), info(), exists(), find(), S3File append, AioS3FileSystem delegation and the tests. Also checked name normalization, explicit/null versions, prefix-only paths, eviction scope, request counts and docstrings against the merge-base.
Result: CLEAN.
- Both stale-listing paths are fixed by the shared eviction in
_head_object(). - A missing object that the listing does not list keeps the listing, and a discovered prefix evicts it.
- The eviction adds no S3 requests.
- The new
ls/version_awarecases fail on 1b26dd1. The prefix and non-matching-entry tests guard behavior that moved. - The fetch/invalidation race stays out of scope as agreed.
Independent review complete for head fed2d2a.
There was a problem hiding this comment.
Repair eebe98d099d4328a8529ce6642edd0117bcbb813: pre-existing finding 3 folded in at the maintainer's request. info() now checks the root path before parse_path().
Self-review of the repair:
- Round 1 (behavior):
info("")/("/")/("s3://")return the root directory object (bucket="", name"") without requests, so fsspecisdir()is True,isfile()False andsize()0. Before, the first two raised ValueError.exists()already returned True for the root. Non-root paths are parsed exactly as before.AioS3FileSystem._infodelegates to this method.refresh/version_iddo not apply to the root. - Round 2 (claims): "never reached since 207c8f0" holds. In 207c8f0 and 4fbced8,
parse_path()precedes the root check, andPATTERN_PATHrequires a bucket of at least one character, so""and"/"never matched. The docstring now states the root is a directory, and the PR body lists the change.
Tests: test_info_root[|/|s3://] fails on fed2d2a and passes here. just lint passed; live tests/pyathena/filesystem 431 passed at eebe98d. An independent follow-up is requested next.
There was a problem hiding this comment.
Independent follow-up (relayed): Codex CLI 0.160.0, model gpt-6-astra, session 01a101d5-146d-79c0-9d75-de6020255926. Static, read-only review of repair eebe98d099d4328a8529ce6642edd0117bcbb813 (git diff fed2d2a0..eebe98d0, git range-diff 9b2f0337..fed2d2a0 9b2f0337..eebe98d0; the earlier commits are unchanged). Afterwards the snapshot was clean and still at the same HEAD.
Covered: root normalization, S3Object construction, non-root parsing/version/cache paths, and fsspec 2026.9.0 stat/isdir/isfile/size/sizes/checksum/ukey/du. Also AioS3FileSystem._info delegation, DirFileSystem, root exists/ls/find/walk/glob/expand_path, the test, the docstring and the parser history.
Result: CLEAN.
- The pre-existing defect is resolved, and non-root behavior is unchanged.
- Root traversal stays consistent with the merge-base:
find/wildcard glob/recursive expansion still reject the root. A literal root glob withdetail=Truenow succeeds. - The unreachability claim holds:
PATTERN_PATHis unchanged since 207c8f0 and requires a non-empty bucket. test_info_rootfails without the repair.
Independent review complete for head eebe98d.
A refreshed info() was the only lookup that removed the cached parent listing, so ls(path, refresh=True) of a deleted object, or the version aware lookup of a listed entry, left a listing that still answered for the deleted object. Evict the parent listing where the contradiction is found instead: when HeadObject misses an object that the listing still lists, and when the ListObjectsV2 check finds a key prefix, which nothing else caches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
info() parsed the path before its root branch, and parse_path rejects
the root path, so info(""), info("/") and info("s3://") raised
ValueError while exists() returned True for them. fsspec's isdir() and
size() raised it as well. Check the root path before parsing it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WHAT
Bucket and key lookups in
S3FileSystemnow use the cached listings consistently.info()andexists()(and fsspec'sisdir(),isfile()andsize(), which callinfo()) answer a key path from the cached listing of its parent directory, the(parent, "/")entry thatls()andfind(maxdepth=...)cache. A key that is listed is returned without HeadObject. A key that the complete parent listing does not contain raisesFileNotFoundError(orexists()returns False) without a request.open()then rejects.info()/exists()withrefresh=True,ls(path, refresh=True), or theversion_awarere-head), and when the ListObjectsV2 check ofinfo()finds a key prefix, which nothing else caches. A HeadObject miss for a path that the listing does not list keeps it.open(path, "a")looks up the existing object with HeadObject (info(path, refresh=True)), because the rewritten object keeps the existing ContentType, Metadata and encryption settings, which a listing entry lacks. This replaces the formerexists()+info()pair; an append of an object whose HeadObject result was already cached now sends one more HeadObject.version_aware, a listed file still gets HeadObject to pin its version, as before. A version-qualified path uses only its own cached entry, because listings describe the current versions.info()andisdir()of a bucket that is not in the cached bucket listing send HeadBucket instead of raisingFileNotFoundError. ListBuckets returns only the buckets that the caller owns.info(""),info("/")andinfo("s3://")return the root directory instead of raisingValueError.parse_path()ran before the root branch, which had never been reached since the filesystem was added (207c8f0), so fsspec'sisdir()andsize()of the root raisedValueErrorwhileexists()returned True.invalidate_cache(""),invalidate_cache("/")andinvalidate_cache("s3://")remove the cached bucket listing. Bucket and key paths still keep it.exists()of a bucket returns True when HeadBucket answers 403.makedirs("s3://bucket/prefix", exist_ok=True)andmkdir()of a prefix therefore succeed for such a bucket instead of raisingPermissionError.Reads go through one
dircache.get()each, as in #1002. The race in which a fetch running during an invalidation writes back a stale entry is out of scope.Behavior changes for the release notes:
info(),size()andexists()of a key that a cachedls()/find(maxdepth=...)listing of its parent covers no longer send HeadObject. The result is the listed entry, which has noContentTypeorMetadata, asls(detail=True)entries do. The listing's ETag is still used forIfMatchonopen().info(),exists(),isfile(),size()andopen(), not onlyls()andfind(). An object that something other than this filesystem instance (Athena, another process, another instance) writes after the listing was cached is reported missing untilrefresh=Trueorinvalidate_cache()is used. s3fs behaves the same way. Writes through this instance invalidate the parent listings as before.exists()returns True for a bucket that HeadBucket denies. Measured on 2026-10-03 with valid credentials, HeadBucket returns 403 for an existing bucket in another account (test,amazon,example) and 404 for a missing one. With invalid credentials, HeadBucket returns 403 for every bucket, soexists()also returns True for a missing bucket in that case. The next request fails withPermissionError.WHY
Closes #965, closes #980.
_ls_dirs()caches listings under(path, delimiter)tuple keys (since v3.15.0), but_ls_from_cache()looked up only string keys. Lookups below a bucket never used the listings, and with list-only permissions they failed where a listing could answer them.info()failure was found by the independent review of this PR; it belongs to the same root/bucket lookup inconsistency as Bucket lookups: info() misses buckets absent from ls(''), invalidate_cache() keeps the root listing, exists() raises on 403 #980._ls_from_cache()treated the bucket listing, which holds only owned buckets, as complete.invalidate_cache()stopped before the root key"", andexists()letPermissionErrorfrom HeadBucket escape.TEST
Tested commit: eebe98d.
just lint: passed.uv run --env-file .env pytest -q -n 4 tests/pyathena/filesystem: 431 passed (live S3).tests/pyathena/filesystem/test_s3.py. The following fail on master and pass here:test_info_uses_cached_listingstest_info_prefers_listed_object_to_prefix_of_same_nametest_info_bucket_missing_from_bucket_listingtest_invalidate_cache_root_drops_bucket_listingtest_exists_bucket_access_deniedtest_dir_filesystem, whoseinfo()is now answered from the listingtest_open_append_keeps_metadata_of_listed_object,test_missing_object_drops_cached_parent_listing(exists/ls/version_aware) andtest_refreshed_prefix_drops_cached_parent_listing(fail on 2efc5a1, the first commit of this PR)test_missing_object_keeps_cached_parent_listing_without_itguards the conditional evictiontest_info_root(fails on master and on fed2d2a)test_open_append_lookup_failurenow makesinfo()fail instead ofexists(), which the append no longer calls.test_info_does_not_use_listing_of_path,test_info_version_aware_heads_listed_file,test_exists_version_ignores_cached_parent_listing.exists()has offline coverage only, because the CI account has no bucket of another account to test against.🤖 Generated with Claude Code