pitrou commented on a change in pull request #11977:
URL: https://github.com/apache/arrow/pull/11977#discussion_r770962938
##########
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:
If we know the backend, we only come here if the backend is Minio, and
that is the second situation :-) Perhaps the wording is confusing?
--
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]
