sijie commented on a change in pull request #510: Issue-605 BP-15 New
CreateLedger API
URL: https://github.com/apache/bookkeeper/pull/510#discussion_r140044620
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
##########
@@ -612,6 +624,57 @@ public void asyncReadUnconfirmedEntries(long firstEntry,
long lastEntry, ReadCal
asyncReadEntriesInternal(firstEntry, lastEntry, cb, ctx);
}
+ /**
+ * Read a sequence of entries asynchronously.
+ *
+ * @param firstEntry
+ * id of first entry of sequence
+ * @param lastEntry
+ * id of last entry of sequence
+ */
+ @Override
+ public CompletableFuture<Iterable<LedgerEntry>> read(long firstEntry, long
lastEntry) {
Review comment:
it is a weird pattern on how do we handle callback and completablefuture: we
are using context object for passing completable future.
isn't it clear just pass a completable future or having a completable future
inside the SyncReadCallback? when the callback is triggered, the completable
future is set. the transformation from Enumeration to Iterable can be done
within SyncReadCallback, without attach a new `apply` function.
also, counter should be renamed to ctx at least. the name were left before
introducing `CompletableFuture`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
