kou commented on code in PR #40075:
URL: https://github.com/apache/arrow/pull/40075#discussion_r1490326513
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2055,6 +2035,116 @@ class AzureFileSystem::Impl {
static constexpr auto kTimeNeededForFileOrDirectoryRename =
std::chrono::seconds{3};
public:
+ /// \pre location.container is not empty.
+ /// \pre location.path is not empty.
+ /// \pre location.path does not end with a trailing slash.
+ Status DeleteFileOnFileSystem(const DataLake::DataLakeFileSystemClient&
adlfs_client,
+ const AzureLocation& location, bool
require_file_to_exist,
+ Azure::Nullable<std::string> lease_id = {}) {
+ DCHECK(!location.container.empty());
+ DCHECK(!location.path.empty());
+ DCHECK(!internal::HasTrailingSlash(location.path));
+ auto file_client = adlfs_client.GetFileClient(location.path);
+ DataLake::GetPathPropertiesOptions get_options;
+ get_options.AccessConditions.LeaseId = lease_id;
+ DataLake::DeleteFileOptions delete_options;
+ delete_options.AccessConditions.LeaseId = std::move(lease_id);
+ try {
+ // This is necessary to avoid deletion of directories via DeleteFile.
+ auto properties = file_client.GetProperties(get_options);
+ if (properties.Value.IsDirectory) {
+ return internal::NotAFile(location.all);
+ }
+ auto response = file_client.Delete(delete_options);
+ // 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.
+ /// \pre location.path does not end with a trailing slash.
+ 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());
+ DCHECK(!internal::HasTrailingSlash(location.path));
+ constexpr auto kFileBlobLeaseTime = std::chrono::seconds{15};
+
+ // If the parent directory of a file is not the container itself, there is
a
+ // risk that deleting the file also deletes the *implied directory* -- the
+ // directory that is implied by the existence of the file path.
+ //
+ // In this case, we must ensure that the deletion is not semantically
+ // equivalent to also deleting the directory. This is done by ensuring the
+ // directory marker blob exists before the file is deleted.
+ std::optional<LeaseGuard> file_blob_lease_guard;
+ const auto parent = location.parent();
+ if (!parent.path.empty()) {
+ // We have to check the existence of the file before checking the
+ // existence of the parent directory marker, so we acquire a lease on the
+ // file first.
+ ARROW_ASSIGN_OR_RAISE(auto file_blob_lease_client,
+ AcquireBlobLease(location, kFileBlobLeaseTime,
+
/*allow_missing=*/!require_file_to_exist));
+ if (file_blob_lease_client) {
+ file_blob_lease_guard.emplace(std::move(file_blob_lease_client),
+ kFileBlobLeaseTime);
+ // Ensure the empty directory marker blob exists before the file is
deleted.
+ //
+ // There is not need to hold a lease on the directory marker because if
+ // a concurrent client deletes the directory marker right after we
+ // create it, the file deletion itself won't be the cause of the
directory
+ // deletion. Additionally, the fact that a lease is held on the blob
path
+ // semantically preserves the directory -- its existence is implied
+ // until the blob representing the file is deleted -- even if another
+ // client deletes the directory marker.
+ RETURN_NOT_OK(EnsureEmptyDirExists(container_client, parent,
operation));
+ } else {
+ // File blob doesn't exist and require_file_to_exists is false, so
just return OK.
+ // Trying to delete a file that doesn't exist, costs only one request
to the
+ // server.
+ DCHECK(!require_file_to_exist);
+ return Status::OK();
+ }
+ }
+
+ auto blob_client = container_client.GetBlobClient(location.path);
+ Blobs::DeleteBlobOptions options;
+ if (file_blob_lease_guard) {
+ options.AccessConditions.LeaseId = file_blob_lease_guard->LeaseId();
+ }
+ try {
+ auto response = blob_client.Delete(options);
Review Comment:
Do we need to call `file_blob_lease_guard->BreakBeforeDeletion()` before
`Delete()`?
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -889,6 +889,117 @@ class TestAzureFileSystem : public ::testing::Test {
ASSERT_RAISES(IOError, fs()->DeleteDirContents(directory_path, false));
}
+ void TestDeleteFileAtRoot() {
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file0"));
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file1/"));
+ const auto container_name = PreexistingData::RandomContainerName(rng_);
+ if (WithHierarchicalNamespace()) {
+ ARROW_UNUSED(CreateFilesystem(container_name));
+ } else {
+ ARROW_UNUSED(CreateContainer(container_name));
+ }
+ arrow::fs::AssertFileInfo(fs(), container_name, FileType::Directory);
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name + "/"));
+ }
+
+ void TestDeleteFileAtContainerRoot() {
+ auto data = SetUpPreexistingData();
+
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
data.Path("nonexistent-file/") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file/")));
+
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::File);
+ ASSERT_OK(fs()->DeleteFile(data.ObjectPath()));
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::NotFound);
+
+ if (WithHierarchicalNamespace()) {
+ auto adlfs_client =
+ datalake_service_client_->GetFileSystemClient(data.container_name);
+ CreateFile(adlfs_client, data.kObjectName, PreexistingData::kLoremIpsum);
+ } else {
+ auto container_client = CreateContainer(data.container_name);
+ CreateBlob(container_client, data.kObjectName,
PreexistingData::kLoremIpsum);
+ }
+
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::File);
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.ObjectPath() + "/'"),
+ fs()->DeleteFile(data.ObjectPath() + "/"));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(data.ObjectPath() + "/"));
+ }
+
+ void TestDeleteFileAtSubdirectory(bool create_empty_dir_marker_first) {
+ auto data = SetUpPreexistingData();
+
+ auto setup_dir_file0 = [this, create_empty_dir_marker_first, &data]() {
+ if (WithHierarchicalNamespace()) {
+ ASSERT_FALSE(create_empty_dir_marker_first);
+ auto adlfs_client =
+ datalake_service_client_->GetFileSystemClient(data.container_name);
+ CreateFile(adlfs_client, "dir/file0", PreexistingData::kLoremIpsum);
+ } else {
+ auto container_client = CreateContainer(data.container_name);
+ if (create_empty_dir_marker_first) {
+ CreateBlob(container_client, "dir/", "");
+ }
+ CreateBlob(container_client, "dir/file0",
PreexistingData::kLoremIpsum);
+ }
+ };
+ setup_dir_file0();
+
+ // Trying to delete a non-existing file in an existing directory should
fail
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("dir/nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("dir/nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
+ data.Path("dir/nonexistent-file/") + "'"),
+ fs()->DeleteFile(data.Path("dir/nonexistent-file/")));
Review Comment:
```suggestion
data.Path("dir/nonexistent-dir/") + "'"),
fs()->DeleteFile(data.Path("dir/nonexistent-dir/")));
```
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2544,7 +2635,28 @@ Status AzureFileSystem::DeleteRootDirContents() {
Status AzureFileSystem::DeleteFile(const std::string& path) {
ARROW_ASSIGN_OR_RAISE(auto location, AzureLocation::FromString(path));
- return impl_->DeleteFile(location);
+ if (location.container.empty()) {
+ return Status::Invalid("DeleteFile requires a non-empty path.");
+ }
+ if (internal::HasTrailingSlash(location.path) || location.path.empty()) {
+ // Container paths (locations w/o path) or paths ending with a slash are
not file
+ // paths.
+ return NotAFile(location);
+ }
Review Comment:
Can we use `ValidateFileLocation()`?
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2055,6 +2035,116 @@ class AzureFileSystem::Impl {
static constexpr auto kTimeNeededForFileOrDirectoryRename =
std::chrono::seconds{3};
public:
+ /// \pre location.container is not empty.
+ /// \pre location.path is not empty.
+ /// \pre location.path does not end with a trailing slash.
+ Status DeleteFileOnFileSystem(const DataLake::DataLakeFileSystemClient&
adlfs_client,
+ const AzureLocation& location, bool
require_file_to_exist,
+ Azure::Nullable<std::string> lease_id = {}) {
Review Comment:
When do we need `lease_id`?
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -889,6 +889,117 @@ class TestAzureFileSystem : public ::testing::Test {
ASSERT_RAISES(IOError, fs()->DeleteDirContents(directory_path, false));
}
+ void TestDeleteFileAtRoot() {
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file0"));
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file1/"));
+ const auto container_name = PreexistingData::RandomContainerName(rng_);
+ if (WithHierarchicalNamespace()) {
+ ARROW_UNUSED(CreateFilesystem(container_name));
+ } else {
+ ARROW_UNUSED(CreateContainer(container_name));
+ }
+ arrow::fs::AssertFileInfo(fs(), container_name, FileType::Directory);
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name + "/"));
+ }
+
+ void TestDeleteFileAtContainerRoot() {
+ auto data = SetUpPreexistingData();
+
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
data.Path("nonexistent-file/") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file/")));
Review Comment:
```suggestion
::testing::HasSubstr("Not a regular file: '" +
data.Path("nonexistent-dir/") +
"'"),
fs()->DeleteFile(data.Path("nonexistent-dir/")));
```
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -1528,6 +1639,21 @@ TYPED_TEST(TestAzureFileSystemOnAllScenarios,
DeleteDirContentsFailureNonexisten
this->TestDeleteDirContentsFailureNonexistent();
}
+TYPED_TEST(TestAzureFileSystemOnAllScenarios, DeleteFileAtRoot) {
+ void TestDeleteFileAtRoot();
Review Comment:
```suggestion
this->TestDeleteFileAtRoot();
```
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -889,6 +889,117 @@ class TestAzureFileSystem : public ::testing::Test {
ASSERT_RAISES(IOError, fs()->DeleteDirContents(directory_path, false));
}
+ void TestDeleteFileAtRoot() {
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file0"));
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file1/"));
+ const auto container_name = PreexistingData::RandomContainerName(rng_);
+ if (WithHierarchicalNamespace()) {
+ ARROW_UNUSED(CreateFilesystem(container_name));
+ } else {
+ ARROW_UNUSED(CreateContainer(container_name));
+ }
+ arrow::fs::AssertFileInfo(fs(), container_name, FileType::Directory);
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name + "/"));
+ }
+
+ void TestDeleteFileAtContainerRoot() {
+ auto data = SetUpPreexistingData();
+
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
data.Path("nonexistent-file/") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file/")));
+
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::File);
+ ASSERT_OK(fs()->DeleteFile(data.ObjectPath()));
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::NotFound);
+
+ if (WithHierarchicalNamespace()) {
+ auto adlfs_client =
+ datalake_service_client_->GetFileSystemClient(data.container_name);
+ CreateFile(adlfs_client, data.kObjectName, PreexistingData::kLoremIpsum);
+ } else {
+ auto container_client = CreateContainer(data.container_name);
+ CreateBlob(container_client, data.kObjectName,
PreexistingData::kLoremIpsum);
+ }
+
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::File);
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.ObjectPath() + "/'"),
+ fs()->DeleteFile(data.ObjectPath() + "/"));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(data.ObjectPath() + "/"));
+ }
+
+ void TestDeleteFileAtSubdirectory(bool create_empty_dir_marker_first) {
+ auto data = SetUpPreexistingData();
+
+ auto setup_dir_file0 = [this, create_empty_dir_marker_first, &data]() {
+ if (WithHierarchicalNamespace()) {
+ ASSERT_FALSE(create_empty_dir_marker_first);
+ auto adlfs_client =
+ datalake_service_client_->GetFileSystemClient(data.container_name);
+ CreateFile(adlfs_client, "dir/file0", PreexistingData::kLoremIpsum);
+ } else {
+ auto container_client = CreateContainer(data.container_name);
+ if (create_empty_dir_marker_first) {
+ CreateBlob(container_client, "dir/", "");
+ }
+ CreateBlob(container_client, "dir/file0",
PreexistingData::kLoremIpsum);
+ }
+ };
+ setup_dir_file0();
+
+ // Trying to delete a non-existing file in an existing directory should
fail
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("dir/nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("dir/nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
+ data.Path("dir/nonexistent-file/") + "'"),
+ fs()->DeleteFile(data.Path("dir/nonexistent-file/")));
+
+ // Trying to delete the directory with DeleteFile should fail
+ if (WithHierarchicalNamespace()) {
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.Path("dir") + "'"),
+ fs()->DeleteFile(data.Path("dir")));
+ } else {
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Path does not exist '" +
data.Path("dir") + "'"),
+ fs()->DeleteFile(data.Path("dir")));
+ }
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.Path("dir/") + "'"),
+ fs()->DeleteFile(data.Path("dir/")));
+
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/file0"), FileType::File);
+ ASSERT_OK(fs()->DeleteFile(data.Path("dir/file0")));
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/file0"),
FileType::NotFound);
+
+ // Recreating the file on the same path gurantees leases were properly
released/broken
Review Comment:
How about using different directory instead for it? It'll be cleaner.
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -889,6 +889,117 @@ class TestAzureFileSystem : public ::testing::Test {
ASSERT_RAISES(IOError, fs()->DeleteDirContents(directory_path, false));
}
+ void TestDeleteFileAtRoot() {
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file0"));
+ ASSERT_RAISES(Invalid, fs()->DeleteFile("file1/"));
+ const auto container_name = PreexistingData::RandomContainerName(rng_);
+ if (WithHierarchicalNamespace()) {
+ ARROW_UNUSED(CreateFilesystem(container_name));
+ } else {
+ ARROW_UNUSED(CreateContainer(container_name));
+ }
+ arrow::fs::AssertFileInfo(fs(), container_name, FileType::Directory);
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(container_name + "/"));
+ }
+
+ void TestDeleteFileAtContainerRoot() {
+ auto data = SetUpPreexistingData();
+
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
data.Path("nonexistent-file/") +
+ "'"),
+ fs()->DeleteFile(data.Path("nonexistent-file/")));
+
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::File);
+ ASSERT_OK(fs()->DeleteFile(data.ObjectPath()));
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::NotFound);
+
+ if (WithHierarchicalNamespace()) {
+ auto adlfs_client =
+ datalake_service_client_->GetFileSystemClient(data.container_name);
+ CreateFile(adlfs_client, data.kObjectName, PreexistingData::kLoremIpsum);
+ } else {
+ auto container_client = CreateContainer(data.container_name);
+ CreateBlob(container_client, data.kObjectName,
PreexistingData::kLoremIpsum);
+ }
+
+ arrow::fs::AssertFileInfo(fs(), data.ObjectPath(), FileType::File);
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.ObjectPath() + "/'"),
+ fs()->DeleteFile(data.ObjectPath() + "/"));
+ ASSERT_RAISES(IOError, fs()->DeleteFile(data.ObjectPath() + "/"));
+ }
+
+ void TestDeleteFileAtSubdirectory(bool create_empty_dir_marker_first) {
+ auto data = SetUpPreexistingData();
+
+ auto setup_dir_file0 = [this, create_empty_dir_marker_first, &data]() {
+ if (WithHierarchicalNamespace()) {
+ ASSERT_FALSE(create_empty_dir_marker_first);
+ auto adlfs_client =
+ datalake_service_client_->GetFileSystemClient(data.container_name);
+ CreateFile(adlfs_client, "dir/file0", PreexistingData::kLoremIpsum);
+ } else {
+ auto container_client = CreateContainer(data.container_name);
+ if (create_empty_dir_marker_first) {
+ CreateBlob(container_client, "dir/", "");
+ }
+ CreateBlob(container_client, "dir/file0",
PreexistingData::kLoremIpsum);
+ }
+ };
+ setup_dir_file0();
+
+ // Trying to delete a non-existing file in an existing directory should
fail
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Path does not exist '" +
data.Path("dir/nonexistent-file") +
+ "'"),
+ fs()->DeleteFile(data.Path("dir/nonexistent-file")));
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" +
+ data.Path("dir/nonexistent-file/") + "'"),
+ fs()->DeleteFile(data.Path("dir/nonexistent-file/")));
+
+ // Trying to delete the directory with DeleteFile should fail
+ if (WithHierarchicalNamespace()) {
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.Path("dir") + "'"),
+ fs()->DeleteFile(data.Path("dir")));
+ } else {
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Path does not exist '" +
data.Path("dir") + "'"),
+ fs()->DeleteFile(data.Path("dir")));
+ }
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError, ::testing::HasSubstr("Not a regular file: '" +
data.Path("dir/") + "'"),
+ fs()->DeleteFile(data.Path("dir/")));
+
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/file0"), FileType::File);
+ ASSERT_OK(fs()->DeleteFile(data.Path("dir/file0")));
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/"), FileType::Directory);
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/file0"),
FileType::NotFound);
+
+ // Recreating the file on the same path gurantees leases were properly
released/broken
+ setup_dir_file0();
+
+ arrow::fs::AssertFileInfo(fs(), data.Path("dir/file0"), FileType::File);
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ IOError,
+ ::testing::HasSubstr("Not a regular file: '" + data.Path("dir/file0/")
+ "'"),
+ fs()->DeleteFile(data.Path("dir/file0/")));
Review Comment:
How about swapping these assertions to check `DeleteFile("dir/file0")` while
`dir/file0` exists?
```suggestion
EXPECT_RAISES_WITH_MESSAGE_THAT(
IOError,
::testing::HasSubstr("Not a regular file: '" +
data.Path("dir/file0/") + "'"),
fs()->DeleteFile(data.Path("dir/file0/")));
arrow::fs::AssertFileInfo(fs(), data.Path("dir/file0"), FileType::File);
```
--
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]
