Hi all, during the discussion on the patch for the new LedgerAPI there is an important decision which arose https://github.com/apache/bookkeeper/pull/510
Should we drop the legacy AsyncCallbacks from the new WriteHandler/ReadHandler APIs ? We are introducing CompletableFuture and this is great, but it would be great to drop legacy callbacks too Styles of API: 1) long addEntry(data) throws BKException, InterruptedException 2) void asyncAddEntry(data, AddCallback cb, Object ctx) 3) CompletableFuture<Long> asyncAddEntry(data) the idea is to drop the 2) style of API Style 1) (sync) is very simple and clear, very good for beginners and for simple cases Style 2) would be good but still uses 'legacy' client API package Style 3) leverages CompletableFuture and it is the new API a fourth option would be to have a new family of callbacks with a little overhead due to the introduction of wrappers/adapters Style 3 can cover uses of 1) but CompletableFuture#get throws ExecutionException (which will wrap the BKException) and it is not very user-friendly Thoughts ? -- Enrico 2017-09-14 12:54 GMT+02:00 Enrico Olivelli <eolive...@gmail.com>: > This is the PR > https://github.com/apache/bookkeeper/pull/510 > > Enrico > > 2017-09-14 12:51 GMT+02:00 Enrico Olivelli <eolive...@gmail.com>: > >> The patch is ready >> Please checkout an comment and possibly merge :-) >> >> minor details: >> - I have not used the Netty4 "recycler" because we are using directly >> CreateLedgerOp (and similar) so the memory footprint is exactly the same as >> 4.5, we can add it in the future (or backport Yahoo improvements from >> Matteo) >> - I have tried to create a complete Client API, so create/open/delete at >> least >> - You will find that javadocs are minimal, it will be another huge task >> to create all of them. We should add references to the new API for each >> "legacy" method >> - I have added a CompletableFuture API for readEntries too, the change is >> minimal as so the API will "more" consistent from my point of view >> - about the tests: I have added minimal test cases to cover all the >> important aspectes, as we are only "renaming" the low level API existing >> test cases are good >> >> >> This work is actually "blocking" the patch for BP-14, I have already a >> prototype for BP-14 but I would not like to spend too much time at rebasing >> and resolving conflicts >> >> Thank you >> I know it will take time to review >> >> Enrico >> >> >> 2017-09-12 10:09 GMT+02:00 Enrico Olivelli <eolive...@gmail.com>: >> >>> Yep >>> We can review the details directly on the patch. >>> >>> Additionally: >>> I think a separate issue will be created, like "Documentation of the new >>> API on the website" >>> >>> Enrico >>> >>> >>> 2017-09-12 10:07 GMT+02:00 Sijie Guo <guosi...@gmail.com>: >>> >>>> cool. just remember to put the new interfaces under >>>> org.apache.bookkeeper.client.api. so we can separate client and server >>>> module in future. >>>> >>>> - Sijie >>>> >>>> On Tue, Sep 12, 2017 at 1:04 AM, Enrico Olivelli <eolive...@gmail.com> >>>> wrote: >>>> >>>> > OK, as there is not -1 I am marking this proposal as "Accepted". >>>> > >>>> > Thank you Jia and Sijie for voting and for comments from other guys >>>> at the >>>> > meeting. >>>> > >>>> > I wlil create and issue and send a pull request soon. >>>> > It will take some time as we need tons of JavaDocs, we are going to >>>> > introduce many *public *new classes, interfaces and method this time >>>> > >>>> > -- Enrico >>>> > >>>> > >>>> > >>>> > >>>> > 2017-09-11 18:57 GMT+02:00 Sijie Guo <guosi...@gmail.com>: >>>> > >>>> > > Enrico, >>>> > > >>>> > > Feel free to close the vote if there is no -1. BP approval is a lazy >>>> > > approval with no -1. >>>> > > >>>> > > - Sijie >>>> > > >>>> > > On Mon, Sep 11, 2017 at 2:27 AM, Enrico Olivelli < >>>> eolive...@gmail.com> >>>> > > wrote: >>>> > > >>>> > > > Ping >>>> > > > >>>> > > > 2017-09-08 6:28 GMT+02:00 Jia Zhai <zhaiji...@gmail.com>: >>>> > > > >>>> > > > > +1 for the new design. >>>> > > > > >>>> > > > > On Thu, Sep 7, 2017 at 3:49 AM, Enrico Olivelli < >>>> eolive...@gmail.com >>>> > > >>>> > > > > wrote: >>>> > > > > >>>> > > > > > Hi all, >>>> > > > > > I would like to call a vote for this BookKeeper proposal >>>> > > > > > >>>> > > > > > This is the wiki page >>>> > > > > > https://cwiki.apache.org/confluence/display/BOOKKEEPER/ >>>> > > > > > BP-15+New+CreateLedger+API >>>> > > > > > >>>> > > > > > It is a new Client API for creating/opening ledgers. >>>> > > > > > We are going to have separate interfaces for Writers and >>>> Readers >>>> > and >>>> > > we >>>> > > > > are >>>> > > > > > going to have a new fluent buider-style API for creating and >>>> > opening >>>> > > > > > ledgers. >>>> > > > > > >>>> > > > > > No real changes in semantics or in protocols to BookKeeper we >>>> are >>>> > > only >>>> > > > > > introducing a new modern and extensible Client API >>>> > > > > > >>>> > > > > > >>>> > > > > > Regards >>>> > > > > > Enrico Olivelli >>>> > > > > > >>>> > > > > >>>> > > > >>>> > > >>>> > >>>> >>> >>> >> >
