From 00327bfae321a2a372371a1f01b60fc9f3dfda92 Mon Sep 17 00:00:00 2001
From: Benny Halevy <bhalevy@scylladb.com>
Date: Mon, 28 Feb 2022 12:45:04 +0200
Subject: [PATCH] directory_lister: drop abort method

Based on Avi's feedback:
> We generally have a public abort() only if we depend on an external
> event (like data from a tcp socket) that we don't control. But here
> there are no such external events. So why have a public abort() at all?

If needed in the future, we can consider adding
get(abort_source&) to allow aborting get() via
an external event.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
---
 test/boost/lister_test.cc | 26 --------------------------
 utils/lister.cc           |  4 ----
 utils/lister.hh           |  6 +-----
 3 files changed, 1 insertion(+), 35 deletions(-)

diff --git a/test/boost/lister_test.cc b/test/boost/lister_test.cc
index 7fe70505d0..3bc77afb9a 100644
--- a/test/boost/lister_test.cc
+++ b/test/boost/lister_test.cc
@@ -191,32 +191,6 @@ SEASTAR_TEST_CASE(test_directory_lister_close) {
     }
 }
 
-SEASTAR_TEST_CASE(test_directory_lister_abort) {
-    auto tmp = tmpdir();
-
-    std::unordered_set<std::string> file_names;
-    std::unordered_set<std::string> dir_names;
-
-    auto count = co_await generate_random_content(tmp, file_names, dir_names, tests::random::get_int(100, 1000));
-    BOOST_TEST_MESSAGE(fmt::format("Generated {} dir entries", count));
-
-    auto dl = directory_lister(tmp.path());
-
-    auto initial = tests::random::get_int(count);
-    BOOST_TEST_MESSAGE(fmt::format("Getting {} dir entries", initial));
-    for (auto i = 0; i < initial; i++) {
-        auto de = co_await dl.get();
-        BOOST_REQUIRE(de);
-    }
-
-    BOOST_TEST_MESSAGE("Aborting directory_lister");
-    dl.abort(std::make_exception_ptr(expected_exception()));
-    BOOST_REQUIRE_THROW(co_await dl.get(), expected_exception);
-
-    BOOST_TEST_MESSAGE("Closing directory_lister");
-    co_await dl.close();
-}
-
 SEASTAR_TEST_CASE(test_directory_lister_extra_get) {
     auto tmp = tmpdir();
 
diff --git a/utils/lister.cc b/utils/lister.cc
index 623fe716b4..3c366416c4 100644
--- a/utils/lister.cc
+++ b/utils/lister.cc
@@ -117,10 +117,6 @@ future<std::optional<directory_entry>> directory_lister::get() {
     co_return std::nullopt;
 }
 
-void directory_lister::abort(std::exception_ptr ex) {
-    _queue.abort(std::move(ex));
-}
-
 future<> directory_lister::close() noexcept {
     if (!_opt_done_fut) {
         return make_ready_future<>();
diff --git a/utils/lister.hh b/utils/lister.hh
index 457651d295..5620cb0899 100644
--- a/utils/lister.hh
+++ b/utils/lister.hh
@@ -190,12 +190,8 @@ public:
     // result has been returned results in a broken_pipe_exception.
     future<std::optional<directory_entry>> get();
 
-    // Abort the directory_lister with a given exception
-    // Further calls to get() will fail with this exception.
-    void abort(std::exception_ptr ex = std::make_exception_ptr(broken_pipe_exception()));
-
     // Close the directory_lister, ignoring any errors.
-    // Must be called after abort() or if get() did not drain the queue.
+    // Must be called after get() if not all entries were retrieved.
     //
     // Close aborts the lister, waking up get() if any is waiting,
     // and waits for all background work to complete.