Count S3FileSystem.find() maxdepth levels like fsspec

[laughingman7743]

WHAT

S3FileSystem.find() (and AioS3FileSystem._find(), which calls it) now counts maxdepth the way fsspec does:

maxdepth	Before	After (fsspec)
0 or less	entries directly under the path	ValueError("maxdepth must be at least 1")
1	also the entries one level below	entries directly under the path
n	n + 1 levels	n levels

_find() now recurses only while more than one level remains, and rejects maxdepth < 1 before listing anything.
The find() docstring describes the level counting and the ValueError, and _find() has a docstring.

Release note (4.0.0, behavior change)

• S3FileSystem.find(maxdepth=n) lists n levels instead of n + 1, and maxdepth=0 raises ValueError. Callers that passed maxdepth relying on the old numbering need maxdepth + 1 to keep their results. S3FileSystem.rm(), glob(), du(), expand_path(), copy(), and get() (and their AioS3FileSystem counterparts except _rm()) pass maxdepth through find(), so S3FileSystem.rm(path, recursive=True, maxdepth=1) no longer deletes the objects one level below the path.

This is a 4.0.0 change only; 3.x keeps the old numbering.

WHY

Fixes #933.

fsspec documents maxdepth as "the maximum number of levels to descend", and its walk(), glob(), and expand_path() reject maxdepth < 1.
fsspec's glob(), expand_path(), and du() call find() with that meaning, so the extra level reached the callers too.
On master, S3FileSystem.rm(path, recursive=True, maxdepth=1) expanded to the objects one level below the path and deleted them, and glob("dir/**", maxdepth=1) returned them.
For example, with the keys d/a.csv, d/sub/c.csv, and d/sub/deep/e.csv, rm("s3://bucket/d", recursive=True, maxdepth=1) sent DeleteObjects for d, d/a.csv, d/sub, d/sub/c.csv, and d/sub/deep, deleting d/sub/c.csv outside the requested depth; with this change it sends d, d/a.csv, and d/sub.
This dates from the maxdepth support added in v3.15.0 (681e749); before that, maxdepth was ignored.

TEST

Tested at 54660a4.

• just lint: passed.
• TestS3FileSystem::test_find_maxdepth_counts_levels_like_fsspec (new, no AWS): a three-level mocked listing; checks that maxdepth=0 raises before any request, and the results of maxdepth=1 (with and without withdirs), 2, and 3. On master it fails; with this change it passes.
• TestS3FileSystem::test_find_refresh_bypasses_cached_listings: uses maxdepth=2 to keep reaching the subdirectory listing it checks.
• test_find_maxdepth (real S3, sync and async): updated to the fsspec numbering, plus maxdepth=0 raising.
• uv run --env-file .env pytest -n 1 tests/pyathena/filesystem/test_s3.py tests/pyathena/filesystem/test_s3_async.py -k "find or glob or rm or walk or du or expand": 27 passed.
• An offline probe with a mocked three-level listing: expand_path(path, recursive=True, maxdepth=1) (used by rm()) and glob("dir/**", maxdepth=1) no longer include the second level, and a no-match prefixed _find(maxdepth=1) (as fsspec's async _glob() sends) still issues one request.

Not run locally: the rest of the filesystem tests and the other suites; CI runs them once the PR is Ready.

Not changed (pre-existing, filed separately): AioS3FileSystem._rm() ignores maxdepth (#962), withdirs=True omits the root (#963), a prefix containing / bypasses the depth limit (#964), info() does not use cached listings (#965), and find(object_path, maxdepth=n) returns [] (#966).

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (implementation behavior): FINDINGS (repaired)

Base e0e85da09435257a18c8b8832b3e10ba7840b697, head 8f29bebed998ae97316a24f28a15fd6f6a1a3a89; full diff (pyathena/filesystem/s3.py, tests/pyathena/filesystem/test_s3.py, tests/pyathena/filesystem/test_s3_async.py).

Covered:

• Level counting against fsspec 2026.9.0 walk()/find(): maxdepth=1 lists only the current level, and each recursion passes maxdepth - 1 >= 1, so the new < 1 check never fires inside the recursion.
• Callers that pass maxdepth through find(): fsspec glob() (its computed depth is at least 1 whenever maxdepth >= 1), expand_path() (used by S3FileSystem.rm() and fsspec copy()/get()/put()), du(), and the async _glob()/_expand_path()/_du() through AioS3FileSystem._find(). A ValueError from the sync _find() propagates through asyncio.to_thread.
• Object-path fallback: it runs only when the current level lists nothing and the path has a key, and it returns only a file. A directory result from info() therefore cannot make the loop recurse into the same path. A subdirectory that becomes empty between listings costs one extra HEAD request and returns []. With prefix, it behaves the same as the unlimited branch (see the updated docstring).
• Tests: the new unit tests fail on master and pass here; test_find_refresh_bypasses_cached_listings still reaches the subdirectory listing with maxdepth=2.

Findings:

1. The new unit test compared find() results in listing order, which comes from _ls_dirs (common prefixes before contents) and is not a contract. Repaired in c1292e0: the multi-entry assertions compare sorted lists.
2. Pre-existing, out of scope: AioS3FileSystem._rm() (pyathena/filesystem/s3_async.py:178) calls expand_path(p, recursive=recursive) without maxdepth, so maxdepth is ignored and _rm(path, recursive=True, maxdepth=1) deletes everything under the path. This PR does not change it; to be filed separately.
3. PR text: the rm() consequence applies to the sync S3FileSystem.rm() only (see finding 2). The PR body will be corrected in round two's claim audit.

[laughingman7743]

Round one on the repair (54660a4): CLEAN. git range-diff e0e85da0..c1292e03 e0e85da0..54660a49 shows the two reviewed commits unchanged plus one commit. The maxdepth branch is again master's listing loop, with only the < 1 check and the > 1 recursion guard changed. key is still used by the unlimited branch. Removing the tests drops only the fallback coverage; test_find_maxdepth_counts_levels_like_fsspec and the live test_find_maxdepth still fail against the old numbering.

[laughingman7743]

Self-review round two (claims, callers, operations): FINDINGS (PR text corrected)

Base e0e85da09435257a18c8b8832b3e10ba7840b697, head c1292e030417ed8a289f9df333ccb676aa3a668c; full pass over the PR body, commit messages, and changed docstrings.

Claims checked:

• Old numbering (0 gave the current level, n gave n + 1 levels, negative values behaved like 0): reproduced with a mocked three-level listing on master; maxdepth > 0 guarded the recursion.
• fsspec 2026.9.0 documents maxdepth as "the maximum number of levels to descend" (find), and walk(), glob(), and expand_path() raise ValueError("maxdepth must be at least 1"): confirmed in fsspec/spec.py.
• ValueError before any request: the unit test asserts _call was not called.
• Object path returned with maxdepth, as fsspec find() does (if not out and self.isfile(path)) and as the unlimited branch already did: confirmed. The prefix docstring no longer limits this to calls without maxdepth.
• Introduced in v3.15.0 (681e749), and before that maxdepth was ignored (# TODO: Support maxdepth and withdirs, full listing): confirmed with git tag --contains and the parent revision.
• maxdepth reaches find() from S3FileSystem.rm() (via expand_path), fsspec glob()/du()/copy()/get(), and the async _glob()/_du()/_expand_path()/_copy()/_get(): confirmed. No other PyAthena module passes maxdepth or calls find() with it.

Corrections to the PR body:

1. put() was listed among the callers, but fsspec put() expands the local path with LocalFileSystem.expand_path(), so S3FileSystem.find() is not involved. Removed.
2. The rm() consequence holds for the sync S3FileSystem.rm() only; AioS3FileSystem._rm() ignores maxdepth (pre-existing, s3_async.py:178). Qualified, and recorded under "Not changed".
3. "Tested at" now separates the live run (8f29beb) from the assertion-only repair (c1292e0, offline tests rerun).

Caller/operator effects: callers that passed maxdepth=0 now get ValueError, and callers relying on the old numbering need maxdepth + 1; both are in the release note. S3 requests drop by one listing level per call. The object-path fallback adds at most one info() (HEAD, then a 1-key listing on 404) when the top-level listing is empty, the same cost as the unlimited branch.

Evidence limits: live S3 coverage is the targeted -n 1 run (28 tests, sync and async) at 8f29beb; the full filesystem and other suites run in CI after Ready.

[laughingman7743]

Round two on the repair (54660a4): FINDINGS (PR text corrected). The PR body no longer claims the object-path behavior and lists #962-#966 under "Not changed". It states the 4.0.0-only target (milestone set on #933 and this PR), and "Tested at" now names 54660a4 with the 27-test live run. Request-count claim: back to master's single listing for a no-match prefixed maxdepth search (mocked measurement). Commit 8f29beb's message still mentions the fallback; 54660a4's message records its removal, and the history is not rewritten.

[laughingman7743]

Independent review (relayed): FINDINGS

Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, sandbox read-only, session 01a10061-dcb1-73f2-8ccb-4a2717704c21. Static review only (no builds, tests, or GitHub access). Base e0e85da09435257a18c8b8832b3e10ba7840b697, head c1292e030417ed8a289f9df333ccb676aa3a668c, detached snapshot; the prompt contained the literal diff, the source snapshot, and the resolved fsspec 2026.9.0 source, with no PR text, commit messages, or prior findings. The snapshot and the PR worktree were unchanged afterwards.

Covered: both find branches, _ls_dirs/info caching, prefix/refresh, object/empty/missing paths, fsspec find/walk, the callers (glob, expand_path, du, copy, get, rm, async delegation), and the mocked/AWS tests. The reviewer found the depth correction sound and noted that the tests fail against the old behavior.

Findings and author verification:

1. P1, pre-existing: AioS3FileSystem._rm() (s3_async.py:191) never passes maxdepth to expand_path, so _rm("bucket/dir", recursive=True, maxdepth=1) deletes everything under the path. Verified; same as round one. Not changed here; to be filed.
2. P2, introduced: this object-path fallback. fsspec's async _glob() passes the filename stem as prefix, so a no-match async glob such as dir/no-match* reaches this branch with an empty listing and now calls info(). If HEAD is denied while LIST is allowed, the call raises PermissionError instead of returning []. Verified: with a mocked listing, _find("bucket/dir/", maxdepth=1, prefix="no-match") issues 3 requests (LIST, HEAD, LIST) instead of master's 1; the permission case needs a HEAD 403 for a missing key while LIST is allowed. Repair: the fallback is removed from this PR, and the find(object, maxdepth=n) inconsistency is to be tracked in its own issue.
3. P2, pre-existing: withdirs=True omits the search root (s3.py:766, s3.py:790), which fsspec includes. Verified, in both branches. To be filed.
4. P2, pre-existing: a prefix containing / bypasses the depth limit (s3.py:751), e.g. find("bucket/dir", maxdepth=1, prefix="sub/deep/") returns a third-level file. Verified; fsspec's glob() passes only a slash-free stem, so this affects direct callers. To be filed.
5. P2, pre-existing: info() cannot use listings cached by _ls_dirs (s3.py:1964): _ls_from_cache reads string keys only, while listings are stored under (path, delimiter). Verified. To be filed.

[laughingman7743]

Repair (54660a4) for finding 2: the object-path fallback is removed from the maxdepth branch, and the prefix docstring is restored to the master wording ("Without maxdepth, ..."). test_find_maxdepth_returns_object_path and the object-path assertions in the sync/async test_find_maxdepth are removed with it. The find(object_path, maxdepth=n) inconsistency is now #966.

Pre-existing findings filed: 1 → #962, 3 → #963, 4 → #964, 5 → #965.

Validation at 54660a4: just lint passed; the offline find tests pass; the mocked no-match prefixed _find("bucket/dir/", maxdepth=1, prefix="no-match") is back to 1 request (3 at c1292e0); uv run --env-file .env pytest -n 1 tests/pyathena/filesystem/test_s3.py tests/pyathena/filesystem/test_s3_async.py -k "find or glob or rm or walk or du or expand": 27 passed.

An independent follow-up on the repair is pending.

[laughingman7743]

Independent follow-up on the repair (relayed): CLEAN

Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, sandbox read-only, session 01a10076-12a4-7471-9e5f-fab69886d38e. Static review only. Scope: git range-diff e0e85da0..c1292e03 e0e85da0..54660a49 (the two reviewed commits unchanged, plus 54660a4), the added commit's diff, and the full base..head diff, on a detached snapshot at 54660a49c40f8e90ba8d93272cfe18ed2f068641; the known pre-existing issues (#962-#966) were excluded.

Covered: S3FileSystem._find/find, AioS3FileSystem._find, fsspec 2026.9.0 sync/async glob/expand_path/du, the remaining tests, and the changed docstrings. Reported: removing the fallback restores the base behavior for empty bounded listings; literal glob and recursive expand_path keep their existence checks; the remaining unit test and the sync/async test_find_maxdepth still detect the original off-by-one, and zero-depth rejection stays covered; the restored prefix wording accurately limits the object fallback to unlimited-depth searches.

The snapshot and the PR worktree were unchanged afterwards (HEAD 54660a49, clean).