ARROW-14916: [C++] GcsFileSystem can delete directories#11890
ARROW-14916: [C++] GcsFileSystem can delete directories#11890coryan wants to merge 6 commits intoapache:masterfrom coryan:ARROW-14916-gcsfs-delete-dir
Conversation
pitrou
left a comment
pitrou
left a comment
There was a problem hiding this comment.
Thanks for the update, just a couple more suggestions.
cpp/src/arrow/filesystem/gcsfs.cc
Outdated
|
|
||
| auto async_delete = | ||
| [&p](gcs::Client& client, | ||
| google::cloud::StatusOr<gcs::ObjectMetadata> o) -> google::cloud::Status { |
There was a problem hiding this comment.
Two suggestions:
- just capture the client as a closure parameter
- return an arrow
Statushere, such that you can useAllFinished(const std::vector<Future>&)
There was a problem hiding this comment.
Done. I captured this because capturing client_ is not possible in C++11.
cpp/src/arrow/filesystem/gcsfs.cc
Outdated
| // This iterates over all the objects, and schedules parallel deletes. | ||
| auto prefix = p.object.empty() ? gcs::Prefix() : gcs::Prefix(p.object); | ||
| for (auto& o : client_.ListObjects(p.bucket, prefix)) { | ||
| submitted.push_back( |
There was a problem hiding this comment.
By using DeferNotOk (which turns a Result<Future<T>> into a Future<T>), you could instead append to a std::vector<Future>, which will simplify the collection code below.
For example (untested):
std::vector<Future> submitted;
// This iterates over all the objects, and schedules parallel deletes.
auto prefix = p.object.empty() ? gcs::Prefix() : gcs::Prefix(p.object);
for (auto& o : client_.ListObjects(p.bucket, prefix)) {
submitted.push_back(DeferNotOk(
io_context.executor()->Submit(async_delete, std::ref(client_), std::move(o))));
}
return AllFinished(submitted).status();| arrow::fs::AssertFileInfo(fs.get(), filename, FileType::NotFound); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Should you test that other paths in the bucket still exist?
|
Benchmark runs are scheduled for baseline = 3ef4032 and contender = fdc344b. fdc344b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
No description provided.