felipecrv commented on code in PR #40075:
URL: https://github.com/apache/arrow/pull/40075#discussion_r1491878105
##########
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:
Same checks, but different errors returned. Anyways, I changed the logic
even more now and the trailing slashes are handled inside
`DeleteFileOnFileSystem/DeleteFileOnContainer` instead.
##########
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:
I removed it in the latest commits.
--
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]
