felipecrv commented on code in PR #40075:
URL: https://github.com/apache/arrow/pull/40075#discussion_r1494515619


##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2055,6 +2057,148 @@ class AzureFileSystem::Impl {
   static constexpr auto kTimeNeededForFileOrDirectoryRename = 
std::chrono::seconds{3};
 
  public:
+  /// \pre location.container is not empty.
+  /// \pre location.path is not empty.
+  Status DeleteFileOnFileSystem(const DataLake::DataLakeFileSystemClient& 
adlfs_client,
+                                const AzureLocation& location,
+                                bool require_file_to_exist) {
+    DCHECK(!location.container.empty());
+    DCHECK(!location.path.empty());
+    auto path_no_trailing_slash =
+        std::string{internal::RemoveTrailingSlash(location.path)};
+    auto file_client = adlfs_client.GetFileClient(path_no_trailing_slash);
+    try {
+      // This is necessary to avoid deletion of directories via DeleteFile.
+      auto properties = file_client.GetProperties();
+      if (properties.Value.IsDirectory) {
+        return internal::NotAFile(location.all);
+      }
+      if (internal::HasTrailingSlash(location.path)) {
+        return internal::NotADir(location.all);
+      }
+      auto response = file_client.Delete();
+      // Only the "*IfExists" functions ever set Deleted to false.
+      // All the others either succeed or throw an exception.
+      DCHECK(response.Value.Deleted);
+    } catch (const Storage::StorageException& exception) {
+      if (exception.StatusCode == Http::HttpStatusCode::NotFound) {
+        // ErrorCode can be "FilesystemNotFound", "PathNotFound"...
+        if (require_file_to_exist) {
+          return PathNotFound(location);
+        }
+        return Status::OK();
+      }
+      return ExceptionToStatus(exception, "Failed to delete a file: ", 
location.path,
+                               ": ", file_client.GetUrl());
+    }
+    return Status::OK();
+  }
+
+  /// \pre location.container is not empty.
+  /// \pre location.path is not empty.
+  Status DeleteFileOnContainer(const Blobs::BlobContainerClient& 
container_client,
+                               const AzureLocation& location, bool 
require_file_to_exist,
+                               const char* operation) {
+    DCHECK(!location.container.empty());
+    DCHECK(!location.path.empty());
+    constexpr auto kFileBlobLeaseTime = std::chrono::seconds{15};
+    auto no_trailing_slash_location = location.RemoveTrailingSlash(
+        /*preserve_original=*/true);

Review Comment:
   Test failures when I remove this:
   
   ```cpp
   [ RUN      ] TestAzureFileSystemOnAllScenarios/2.DeleteFileAtContainerRoot
   /Users/felipe/code/arrow/cpp/src/arrow/filesystem/azurefs_test.cc:942: 
Failure
   Failed
   'fs()->DeleteFile(data.ObjectPath() + "/")' did not fail with errno=ENOTDIR: 
IOError: Path does not exist 
'w3ay9l35qoxg0i0lqbfababvwbrrrhxq/test-object-name/'. Detail: [errno 2] No such 
file or directory
   
   [  FAILED  ] TestAzureFileSystemOnAllScenarios/2.DeleteFileAtContainerRoot, 
where TypeParam = arrow::fs::TestingScenario<arrow::fs::AzureFlatNSEnv,true> 
(3185 ms)
   [ RUN      ] TestAzureFileSystemOnAllScenarios/2.DeleteFileAtSubdirectory
   /Users/felipe/code/arrow/cpp/src/arrow/filesystem/azurefs_test.cc:1000: 
Failure
   Value of: _st.ToStringWithoutContextLines()
   Expected: has substring "Not a directory: 
'sco3n8q67r3cd2mas6lx53nlc36uw7f9/dir/file0/'"
     Actual: "IOError: Path does not exist 
'sco3n8q67r3cd2mas6lx53nlc36uw7f9/dir/file0/'. Detail: [errno 2] No such file 
or directory"
   
   /Users/felipe/code/arrow/cpp/src/arrow/filesystem/azurefs_test.cc:1000: 
Failure
   Value of: _st.ToStringWithoutContextLines()
   Expected: has substring "Not a directory: 
'3nw7fyp8whjb3r8e6g7eoyg2zav8nr8y/dir/file0/'"
     Actual: "IOError: Path does not exist 
'3nw7fyp8whjb3r8e6g7eoyg2zav8nr8y/dir/file0/'. Detail: [errno 2] No such file 
or directory"
   
   [  FAILED  ] TestAzureFileSystemOnAllScenarios/2.DeleteFileAtSubdirectory, 
where TypeParam = arrow::fs::TestingScenario<arrow::fs::AzureFlatNSEnv,true> 
(12108 ms)
   ```
   
   I'm going to implement this by copying `location` and removing the trailing 
slashes manually.



-- 
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]

Reply via email to