You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/22 15:33:31 UTC

[GitHub] [arrow] westonpace commented on a diff in pull request #12943: ARROW-16261: [C++] Fix DeleteDirContents on HDFS with missing_dir_ok=True

westonpace commented on code in PR #12943:
URL: https://github.com/apache/arrow/pull/12943#discussion_r856343619


##########
cpp/src/arrow/filesystem/hdfs.cc:
##########
@@ -194,28 +195,33 @@ class HadoopFileSystem::Impl {
     return Status::OK();
   }
 
-  Status DeleteDir(const std::string& path) {
-    if (!IsDirectory(path)) {
-      return Status::IOError("Cannot delete directory '", path, "': not a directory");
+  Status CheckForDirectory(const std::string& path, const char* action) {
+    // Check existence of path, and that it's a directory
+    io::HdfsPathInfo info;
+    RETURN_NOT_OK(client_->GetPathInfo(path, &info));
+    if (info.kind != io::ObjectType::DIRECTORY) {
+      return Status::IOError("Cannot ", action, " directory '", path,
+                             "': not a directory");
     }
-    RETURN_NOT_OK(client_->DeleteDirectory(path));
     return Status::OK();
   }
 
+  Status DeleteDir(const std::string& path) {
+    RETURN_NOT_OK(CheckForDirectory(path, "delete"));
+    return client_->DeleteDirectory(path);
+  }
+
   Status DeleteDirContents(const std::string& path, bool missing_dir_ok) {
-    if (!IsDirectory(path)) {
-      return Status::IOError("Cannot delete contents of directory '", path,
-                             "': not a directory");
-    }
-    std::vector<std::string> file_list;
-    Status get_children_st = client_->GetChildren(path, &file_list);
-    if (!get_children_st.ok()) {
-      if (missing_dir_ok &&
-          ::arrow::internal::ErrnoFromStatus(get_children_st) == ENOENT) {
+    auto st = CheckForDirectory(path, "delete contents of ");

Review Comment:
   ```suggestion
       auto st = CheckForDirectory(path, "delete contents of");
   ```



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org