Skip to content

Commit fee511b

Browse files
docs(io): simplify DeleteFiles comments
1 parent 10f8765 commit fee511b

2 files changed

Lines changed: 4 additions & 7 deletions

File tree

‎src/iceberg/arrow/s3/arrow_s3_file_io.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,8 @@ Status ArrowS3FileIO::DeleteFile(const std::string& file_location) {
284284
}
285285

286286
Status ArrowS3FileIO::DeleteFiles(const std::vector<std::string>& file_locations) {
287-
// Like Java's S3FileIO, a failure neither stops the remaining deletes nor
288-
// hides how many failed. One at a time costs nothing extra: Arrow's S3
289-
// DeleteFiles deletes each file separately as well.
287+
// Like Java's S3FileIO, keep going after a failure and report the count.
288+
// Arrow's S3 DeleteFiles deletes one file at a time too.
290289
size_t failed = 0;
291290
for (const auto& file_location : file_locations) {
292291
if (auto status = FileIOForPath(file_location).DeleteFile(file_location);

‎src/iceberg/test/arrow_s3_file_io_test.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,7 @@ TEST_F(ArrowS3FileIOTest, DeleteFilesAttemptsEveryFile) {
322322
ASSERT_THAT(io->WriteFile(path, "payload"), IsOk());
323323
}
324324

325-
// Deletes under `denied` now authenticate with bad keys and fail; the one in
326-
// between must still run.
325+
// Deletes under `denied` fail; the one between them must still run.
327326
auto bad_properties = properties;
328327
for (const auto& [key, value] : BadS3Credentials()) {
329328
bad_properties.insert_or_assign(key, value);
@@ -334,8 +333,7 @@ TEST_F(ArrowS3FileIOTest, DeleteFilesAttemptsEveryFile) {
334333
EXPECT_THAT(io->DeleteFiles(paths), HasErrorMessage("Failed to delete 2 of 3 files"));
335334
EXPECT_FALSE(io->ReadFile(allowed, std::nullopt).has_value());
336335

337-
// With the default credentials back, the denied files must still exist:
338-
// Arrow fails to delete a missing object.
336+
// The denied files remain: Arrow fails to delete a missing object.
339337
ASSERT_THAT(credentialed->SetStorageCredentials({}), IsOk());
340338
EXPECT_THAT(io->DeleteFiles({paths[0], paths[2]}), IsOk());
341339
}

0 commit comments

Comments
 (0)