felipecrv commented on code in PR #38793:
URL: https://github.com/apache/arrow/pull/38793#discussion_r1399820084
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -507,6 +507,41 @@ TEST_F(AzuriteFileSystemTest, CreateDirUri) {
ASSERT_RAISES(Invalid, fs_->CreateDir("abfs://" + RandomContainerName(),
true));
}
+TEST_F(AzuriteFileSystemTest, DeleteContainerDirSuccess) {
+ auto container_name = RandomContainerName();
+ ASSERT_OK(fs_->CreateDir(container_name));
+ ASSERT_OK(fs_->DeleteDir(container_name));
+}
+
+TEST_F(AzuriteFileSystemTest, DeleteDirSuccess) {
+ const auto path =
+ internal::ConcatAbstractPath(PreexistingContainerName(),
RandomDirectoryName());
+ // There is only virtual directory without hierarchical namespace
+ // support. So the DeleteDir() does nothing.
Review Comment:
Shouldn't it delete all the blobs that start with `path/to/dir/`?
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -724,6 +724,59 @@ class AzureFileSystem::Impl {
return Status::OK();
}
+
+ Status DeleteDir(const AzureLocation& location) {
+ if (location.container.empty()) {
+ return Status::Invalid("Cannot delete an empty container");
+ }
+
+ if (location.path.empty()) {
+ auto container_client =
+ blob_service_client_->GetBlobContainerClient(location.container);
+ try {
+ auto response = container_client.Delete();
+ if (response.Value.Deleted) {
+ return Status::OK();
+ } else {
+ return StatusFromErrorResponse(
+ container_client.GetUrl(), response.RawResponse.get(),
+ "Failed to delete a container: " + location.container);
+ }
+ } catch (const Azure::Storage::StorageException& exception) {
+ return internal::ExceptionToStatus(
+ "Failed to delete a container: " + location.container + ": " +
+ container_client.GetUrl(),
+ exception);
+ }
+ }
+
+ ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled,
+ hierarchical_namespace_.Enabled(location.container));
+ if (!hierarchical_namespace_enabled) {
+ // Without hierarchical namespace enabled Azure blob storage has no
directories.
+ // Therefore we can't, and don't need to delete one.
+ return Status::OK();
+ }
Review Comment:
I agree with @Tom-Newton here. Azure API might have an endpoint to delete
all blobs with a certain prefix, so we don't necessarily have to loop from the
client.
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
