pitrou commented on a change in pull request #11977:
URL: https://github.com/apache/arrow/pull/11977#discussion_r770952807
##########
File path: cpp/src/arrow/filesystem/s3fs.cc
##########
@@ -1720,7 +1720,27 @@ class S3FileSystem::Impl : public
std::enable_shared_from_this<S3FileSystem::Imp
// a non-empty "directory". This is a Minio-specific quirk, but we need
// to handle it for unit testing.
- Status IsEmptyDirectory(const std::string& bucket, const std::string& key,
bool* out) {
+ // If this method is called after HEAD on "bucket/key" already returned a
404,
+ // can pass the given outcome to spare a spurious HEAD call.
+ Result<bool> IsEmptyDirectory(
+ const std::string& bucket, const std::string& key,
+ const S3Model::HeadObjectOutcome* previous_outcome = nullptr) {
Review comment:
> Generally, use std::optional to represent optional by-value inputs,
and use a const pointer when the non-optional form would have used a reference.
Here, the non-optional form would have used a reference (we don't want to
copy the entire outcome object).
--
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]