westonpace commented on a change in pull request #11977:
URL: https://github.com/apache/arrow/pull/11977#discussion_r770961026
##########
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) {
+ if (previous_outcome) {
+ // Fetch the backend from the previous error
+ DCHECK(!previous_outcome->IsSuccess());
+ if (!backend_) {
+ SaveBackend(previous_outcome->GetError());
+ DCHECK(backend_);
+ }
+ if (backend_ != S3Backend::Minio) {
+ // HEAD already returned a 404, nothing more to do
+ return false;
+ }
+ }
+
+ // We come here in one of two situations:
+ // - we don't know the backend and there is no previous outcome
Review comment:
The working sounds like "we don't know the backend" (A) and "there is no
previous outcome" (B) both have to be true (A & B). But it seems like we would
get here on (!A & B) as well.
--
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
