pitrou commented on code in PR #38845:
URL: https://github.com/apache/arrow/pull/38845#discussion_r1413710622
##########
python/pyarrow/tests/test_fs.py:
##########
@@ -760,6 +760,34 @@ def test_delete_dir(fs, pathfn):
fs.delete_dir(d)
+def test_delete_dir_with_explicit_subdir(fs, pathfn):
+ skip_fsspec_s3fs(fs)
Review Comment:
Also, mention the GH issue and explain what this test is about? Otherwise,
it's not obvious why the test is needed at all.
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -2408,7 +2408,12 @@ class S3FileSystem::Impl : public
std::enable_shared_from_this<S3FileSystem::Imp
std::vector<std::string> file_paths;
for (const auto& file_info : file_infos) {
DCHECK_GT(file_info.path().size(), bucket.size());
- file_paths.push_back(file_info.path().substr(bucket.size()
+ 1));
+ auto file_path = file_info.path().substr(bucket.size() +
1);
+ if (file_info.IsDirectory()) {
+ DCHECK_OK(internal::AssertNoTrailingSlash(file_path));
+ file_path = file_path + kSep;
Review Comment:
Can you please add a comment here referring to the GH issue? Otherwise this
will easily get lost.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
