Code review fixes

Signed-off-by: Joaquin Anton <[email protected]>
NVIDIA · Apr 22, 2024 · 1ae1075 · 1ae1075
1 parent 684aaee
commit 1ae1075
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 9 deletions.
diff --git a/dali/util/s3_file.cc b/dali/util/s3_file.cc
@@ -28,9 +28,6 @@ S3FileStream::S3FileStream(Aws::S3::S3Client* s3_client, const std::string& uri,
     object_stats_.size = size.value();
   } else {
     object_stats_ = s3_filesystem::get_stats(s3_client, object_location_);
-    if (!object_stats_.exists)
-      throw std::runtime_error("S3 Object not found. bucket=" + object_location_.bucket +
-                               " object=" + object_location_.object);
   }
 }
 

diff --git a/dali/util/s3_filesystem.cc b/dali/util/s3_filesystem.cc
@@ -56,7 +56,13 @@ S3ObjectStats get_stats(Aws::S3::S3Client* s3_client, const S3ObjectLocation& ob
   head_object_req.SetResponseStreamFactory(
       []() { return Aws::New<Aws::StringStream>(kAllocationTag); });
   auto head_object_outcome = s3_client->HeadObject(head_object_req);
-  stats.exists = head_object_outcome.IsSuccess();
+  if (!head_object_outcome.IsSuccess()) {
+    const Aws::S3::S3Error& err = head_object_outcome.GetError();
+    throw std::runtime_error("S3 Object not found. bucket=" + object_location.bucket +
+                             " object=" + object_location.object + ":\n" + err.GetExceptionName() +
+                             ": " + err.GetMessage());
+  }
+  stats.exists = true;
   stats.size = stats.exists ? head_object_outcome.GetResult().GetContentLength() : 0;
   return stats;
 }
@@ -82,12 +88,12 @@ size_t read_object_contents(Aws::S3::S3Client* s3_client, const S3ObjectLocation
       [&streambuf]() { return Aws::New<Aws::IOStream>(kAllocationTag, &streambuf); });
 
   size_t bytes_read = 0;
-  auto getObjectOutcome = s3_client->GetObject(getObjectRequest);
-  if (!getObjectOutcome.IsSuccess()) {
-    const Aws::S3::S3Error& err = getObjectOutcome.GetError();
+  auto get_object_outcome = s3_client->GetObject(getObjectRequest);
+  if (!get_object_outcome.IsSuccess()) {
+    const Aws::S3::S3Error& err = get_object_outcome.GetError();
     throw std::runtime_error(err.GetExceptionName() + ": " + err.GetMessage());
   } else {
-    bytes_read = getObjectOutcome.GetResult().GetContentLength();
+    bytes_read = get_object_outcome.GetResult().GetContentLength();
   }
   return bytes_read;
 }

diff --git a/dali/util/s3_filesystem.h b/dali/util/s3_filesystem.h
@@ -68,14 +68,15 @@ DLL_PUBLIC size_t read_object_contents(Aws::S3::S3Client* s3_client,
                                        const S3ObjectLocation& object_location, void* buf, size_t n,
                                        size_t offset = 0);
 
+using PerObjectCallable = std::function<void(const std::string&, size_t)>;
+
 /**
  * @brief Visits all objects under a given object location
  *
  * @param s3_client open S3 client
  * @param object_location S3 object location
  * @param per_object_call callable to run on each object listed
  */
-using PerObjectCallable = std::function<void(const std::string&, size_t)>;
 DLL_PUBLIC void list_objects_f(Aws::S3::S3Client* s3_client,
                                const S3ObjectLocation& object_location,
                                PerObjectCallable per_object_call);