From 8f29bebed998ae97316a24f28a15fd6f6a1a3a89 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:05:55 +0900 Subject: [PATCH 1/3] Count S3FileSystem.find() maxdepth levels like fsspec find(path, maxdepth=n) listed n + 1 levels: _find() listed the current level and recursed while maxdepth > 0. fsspec's find() and walk() count the level directly under the path as 1 and reject maxdepth < 1, and glob(), expand_path(), rm(), and du() pass maxdepth on with that meaning. rm(path, recursive=True, maxdepth=1) therefore also deleted the objects one level below the path. Recurse only while more than one level remains, and raise ValueError for maxdepth < 1. With maxdepth, also return the path itself when it is an object, as find() already does without maxdepth. Closes #933 Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 38 +++++++++++-- tests/pyathena/filesystem/test_s3.py | 64 +++++++++++++++++++--- tests/pyathena/filesystem/test_s3_async.py | 20 +++++-- 3 files changed, 105 insertions(+), 17 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index ba7abb984..2d27197c4 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -717,6 +717,24 @@ def _find( withdirs: bool | None = None, **kwargs, ) -> list[S3Object]: + """List the objects below a path, as described in ``find``. + + Args: + path: S3 path to search under. + maxdepth: Maximum number of levels to descend, at least 1 + (None for unlimited). + withdirs: Whether to include directories in the result. + **kwargs: Additional arguments including ``prefix`` and + ``refresh``, as described in ``find``. + + Returns: + The objects found, and the directories if ``withdirs`` is True. + + Raises: + ValueError: If ``maxdepth`` is less than 1 or the path is the root. + """ + if maxdepth is not None and maxdepth < 1: + raise ValueError("maxdepth must be at least 1") path = self._strip_protocol(path) if path in ["", "/"]: raise ValueError("Cannot traverse all files in S3.") @@ -731,6 +749,13 @@ def _find( # List files and directories at current level current_items = self._ls_dirs(path, prefix=prefix, delimiter="/", refresh=refresh) + if not current_items and key: + # The path itself may be an object, as without maxdepth. + try: + info = self.info(path, refresh=refresh) + except FileNotFoundError: + return [] + return [info] if info.type == S3ObjectType.S3_OBJECT_TYPE_FILE else [] for item in current_items: if item.type == S3ObjectType.S3_OBJECT_TYPE_FILE: @@ -742,7 +767,7 @@ def _find( result.append(item) # Recursively explore subdirectory if depth allows - if maxdepth > 0: + if maxdepth > 1: sub_path = f"s3://{bucket}/{item.key}" sub_results = self._find( sub_path, maxdepth=maxdepth - 1, withdirs=withdirs, **kwargs @@ -786,19 +811,24 @@ def find( Args: path: S3 path to search under (e.g., "s3://bucket/prefix"). - maxdepth: Maximum depth to recurse (None for unlimited). + maxdepth: Maximum number of levels to descend, at least 1 + (None for unlimited). With 1, only the entries directly under + the path are listed. withdirs: Whether to include directories in results (None = default behavior). detail: If True, return dict of {path: S3Object}; if False, return list of paths. **kwargs: Additional arguments including: prefix: Key prefix, relative to the path, to filter the listed keys - by. Without maxdepth, if nothing is listed and the path itself is - an object, that object is returned regardless of the prefix. + by. If nothing is listed and the path itself is an object, + that object is returned regardless of the prefix. refresh: If True, bypass the cache and list from S3. Returns: Dictionary mapping paths to S3Objects (if detail=True) or list of paths (if detail=False). + Raises: + ValueError: If ``maxdepth`` is less than 1 or the path is the root. + Example: >>> fs = S3FileSystem() >>> fs.find("s3://bucket/data/", maxdepth=2) # Limit depth diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 2d0fa6cc0..0d52700e3 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -279,7 +279,49 @@ def test_find_refresh_bypasses_cached_listings(self): assert fs.find("s3://bucket/dir", refresh=True) == ["bucket/dir/sub/new"] # The subdirectory listings of maxdepth are refreshed as well. - assert fs.find("s3://bucket/dir", maxdepth=1, refresh=True) == ["bucket/dir/sub/new"] + assert fs.find("s3://bucket/dir", maxdepth=2, refresh=True) == ["bucket/dir/sub/new"] + + def test_find_maxdepth_counts_levels_like_fsspec(self): + fs = self._make_fs() + responses = { + "dir/": { + "Contents": [{"Key": "dir/direct"}], + "CommonPrefixes": [{"Prefix": "dir/sub/"}], + }, + "dir/sub/": { + "Contents": [{"Key": "dir/sub/nested"}], + "CommonPrefixes": [{"Prefix": "dir/sub/deep/"}], + }, + "dir/sub/deep/": {"Contents": [{"Key": "dir/sub/deep/file"}]}, + } + fs._call.side_effect = lambda method, **kwargs: responses[kwargs["Prefix"]] + + with pytest.raises(ValueError, match="maxdepth must be at least 1"): + fs.find("s3://bucket/dir", maxdepth=0) + fs._call.assert_not_called() + + assert fs.find("s3://bucket/dir", maxdepth=1) == ["bucket/dir/direct"] + assert fs.find("s3://bucket/dir", maxdepth=1, withdirs=True) == [ + "bucket/dir/sub", + "bucket/dir/direct", + ] + assert fs.find("s3://bucket/dir", maxdepth=2) == [ + "bucket/dir/sub/nested", + "bucket/dir/direct", + ] + assert fs.find("s3://bucket/dir", maxdepth=3) == [ + "bucket/dir/sub/deep/file", + "bucket/dir/sub/nested", + "bucket/dir/direct", + ] + + def test_find_maxdepth_returns_object_path(self): + fs = self._make_fs() + fs.dircache["bucket/dir/file"] = self._file_object("dir/file") + fs._call.return_value = {} + + assert fs.find("s3://bucket/dir/file", maxdepth=1) == ["bucket/dir/file"] + assert fs.find("s3://bucket/dir/file") == ["bucket/dir/file"] def test_refresh_evicts_cached_object_and_bucket_not_found(self): fs = self._make_fs() @@ -1089,22 +1131,30 @@ def test_find_maxdepth(self, fs): fs.touch(f"{dir_}/level1/level2/file2.txt") fs.touch(f"{dir_}/level1/level2/level3/file3.txt") - # Test maxdepth=0 (only files in the root) - result = fs.find(dir_, maxdepth=0) + # maxdepth must be at least 1, as in fsspec + with pytest.raises(ValueError, match="maxdepth must be at least 1"): + fs.find(dir_, maxdepth=0) + + # Test maxdepth=1 (only files in the root) + result = fs.find(dir_, maxdepth=1) assert len(result) == 1 assert fs._strip_protocol(f"{dir_}/file0.txt") in result - # Test maxdepth=1 (files in root and level1) - result = fs.find(dir_, maxdepth=1) + # Test maxdepth=2 (files in root and level1) + result = fs.find(dir_, maxdepth=2) assert len(result) == 2 assert fs._strip_protocol(f"{dir_}/file0.txt") in result assert fs._strip_protocol(f"{dir_}/level1/file1.txt") in result - # Test maxdepth=2 (files in root, level1, and level2) - result = fs.find(dir_, maxdepth=2) + # Test maxdepth=3 (files in root, level1, and level2) + result = fs.find(dir_, maxdepth=3) assert len(result) == 3 assert fs._strip_protocol(f"{dir_}/level1/level2/file2.txt") in result + # Test maxdepth on an object path (the object itself) + result = fs.find(f"{dir_}/file0.txt", maxdepth=1) + assert result == [fs._strip_protocol(f"{dir_}/file0.txt")] + # Test no maxdepth (all files) result = fs.find(dir_) assert len(result) == 4 diff --git a/tests/pyathena/filesystem/test_s3_async.py b/tests/pyathena/filesystem/test_s3_async.py index fc0c354f5..391bdf040 100644 --- a/tests/pyathena/filesystem/test_s3_async.py +++ b/tests/pyathena/filesystem/test_s3_async.py @@ -442,22 +442,30 @@ async def test_find_maxdepth(self, fs): await fs._touch(f"{dir_}/level1/level2/file2.txt") await fs._touch(f"{dir_}/level1/level2/level3/file3.txt") - # Test maxdepth=0 (only files in the root) - result = await fs._find(dir_, maxdepth=0) + # maxdepth must be at least 1, as in fsspec + with pytest.raises(ValueError, match="maxdepth must be at least 1"): + await fs._find(dir_, maxdepth=0) + + # Test maxdepth=1 (only files in the root) + result = await fs._find(dir_, maxdepth=1) assert len(result) == 1 assert fs._strip_protocol(f"{dir_}/file0.txt") in result - # Test maxdepth=1 (files in root and level1) - result = await fs._find(dir_, maxdepth=1) + # Test maxdepth=2 (files in root and level1) + result = await fs._find(dir_, maxdepth=2) assert len(result) == 2 assert fs._strip_protocol(f"{dir_}/file0.txt") in result assert fs._strip_protocol(f"{dir_}/level1/file1.txt") in result - # Test maxdepth=2 (files in root, level1, and level2) - result = await fs._find(dir_, maxdepth=2) + # Test maxdepth=3 (files in root, level1, and level2) + result = await fs._find(dir_, maxdepth=3) assert len(result) == 3 assert fs._strip_protocol(f"{dir_}/level1/level2/file2.txt") in result + # Test maxdepth on an object path (the object itself) + result = await fs._find(f"{dir_}/file0.txt", maxdepth=1) + assert result == [fs._strip_protocol(f"{dir_}/file0.txt")] + # Test no maxdepth (all files) result = await fs._find(dir_) assert len(result) == 4 From c1292e030417ed8a289f9df333ccb676aa3a668c Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:07:54 +0900 Subject: [PATCH 2/3] Compare find() results without depending on listing order Co-Authored-By: Claude Opus 5.5 --- tests/pyathena/filesystem/test_s3.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 0d52700e3..de718d738 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -301,18 +301,18 @@ def test_find_maxdepth_counts_levels_like_fsspec(self): fs._call.assert_not_called() assert fs.find("s3://bucket/dir", maxdepth=1) == ["bucket/dir/direct"] - assert fs.find("s3://bucket/dir", maxdepth=1, withdirs=True) == [ - "bucket/dir/sub", + assert sorted(fs.find("s3://bucket/dir", maxdepth=1, withdirs=True)) == [ "bucket/dir/direct", + "bucket/dir/sub", ] - assert fs.find("s3://bucket/dir", maxdepth=2) == [ - "bucket/dir/sub/nested", + assert sorted(fs.find("s3://bucket/dir", maxdepth=2)) == [ "bucket/dir/direct", + "bucket/dir/sub/nested", ] - assert fs.find("s3://bucket/dir", maxdepth=3) == [ + assert sorted(fs.find("s3://bucket/dir", maxdepth=3)) == [ + "bucket/dir/direct", "bucket/dir/sub/deep/file", "bucket/dir/sub/nested", - "bucket/dir/direct", ] def test_find_maxdepth_returns_object_path(self): From 54660a49c40f8e90ba8d93272cfe18ed2f068641 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:29:05 +0900 Subject: [PATCH 3/3] Drop the object-path fallback from find() with maxdepth fsspec's async _glob() passes the filename stem as prefix, so a glob without matches reached the fallback with an empty listing and paid a HeadObject and another ListObjectsV2 request for nothing. Keep this change to the maxdepth level counting; find(object_path, maxdepth=n) returning [] is tracked separately. Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 11 ++--------- tests/pyathena/filesystem/test_s3.py | 12 ------------ tests/pyathena/filesystem/test_s3_async.py | 4 ---- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index 2d27197c4..9211d6fde 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -749,13 +749,6 @@ def _find( # List files and directories at current level current_items = self._ls_dirs(path, prefix=prefix, delimiter="/", refresh=refresh) - if not current_items and key: - # The path itself may be an object, as without maxdepth. - try: - info = self.info(path, refresh=refresh) - except FileNotFoundError: - return [] - return [info] if info.type == S3ObjectType.S3_OBJECT_TYPE_FILE else [] for item in current_items: if item.type == S3ObjectType.S3_OBJECT_TYPE_FILE: @@ -818,8 +811,8 @@ def find( detail: If True, return dict of {path: S3Object}; if False, return list of paths. **kwargs: Additional arguments including: prefix: Key prefix, relative to the path, to filter the listed keys - by. If nothing is listed and the path itself is an object, - that object is returned regardless of the prefix. + by. Without maxdepth, if nothing is listed and the path itself is + an object, that object is returned regardless of the prefix. refresh: If True, bypass the cache and list from S3. Returns: diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index de718d738..e0ea0b342 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -315,14 +315,6 @@ def test_find_maxdepth_counts_levels_like_fsspec(self): "bucket/dir/sub/nested", ] - def test_find_maxdepth_returns_object_path(self): - fs = self._make_fs() - fs.dircache["bucket/dir/file"] = self._file_object("dir/file") - fs._call.return_value = {} - - assert fs.find("s3://bucket/dir/file", maxdepth=1) == ["bucket/dir/file"] - assert fs.find("s3://bucket/dir/file") == ["bucket/dir/file"] - def test_refresh_evicts_cached_object_and_bucket_not_found(self): fs = self._make_fs() fs.dircache["bucket/key"] = self._file_object("key") @@ -1151,10 +1143,6 @@ def test_find_maxdepth(self, fs): assert len(result) == 3 assert fs._strip_protocol(f"{dir_}/level1/level2/file2.txt") in result - # Test maxdepth on an object path (the object itself) - result = fs.find(f"{dir_}/file0.txt", maxdepth=1) - assert result == [fs._strip_protocol(f"{dir_}/file0.txt")] - # Test no maxdepth (all files) result = fs.find(dir_) assert len(result) == 4 diff --git a/tests/pyathena/filesystem/test_s3_async.py b/tests/pyathena/filesystem/test_s3_async.py index 391bdf040..cbc383f1f 100644 --- a/tests/pyathena/filesystem/test_s3_async.py +++ b/tests/pyathena/filesystem/test_s3_async.py @@ -462,10 +462,6 @@ async def test_find_maxdepth(self, fs): assert len(result) == 3 assert fs._strip_protocol(f"{dir_}/level1/level2/file2.txt") in result - # Test maxdepth on an object path (the object itself) - result = await fs._find(f"{dir_}/file0.txt", maxdepth=1) - assert result == [fs._strip_protocol(f"{dir_}/file0.txt")] - # Test no maxdepth (all files) result = await fs._find(dir_) assert len(result) == 4