hmm, I actually like the alternative you rejected. Instead, I don't think
the approach in the proposal is the right one that we should take.
1) LedgerConfiguration is a confusing term used here. what doest a
`configuration` mean here, is the configuration stored somewhere or just
purely runtime configuration?
2) It is not an extendible solution. how does this apply to open/delete
operations?
I'd like to extend your alternative to understand why it is rejected by you.
===== The builder approach ==========
1) builder interfaces for operations
- create operations builder interfaces under
org/apache/bookkeeper/client/api/op/
- add a ICreateBuilder for create operation:
interface ICreateBuilder {
ICreateBuilder withEnsembleSize(...);
ICreateBuilder withWriteQuorumSize(...);
...
void execute(CreateCallback callback, Object ctx);
// support java8 completable future
CompletableFuture<LedgerHandle> execute();
}
- LedgerCreateOp implements CreateBuilder
2) create an interface for bookkeeper. IBookKeeper under
org/apache/bookkeeper/client/api/IBookKeeper
interface IBookKeeper {
/**
* new a create ledger builder.
*/
ICreateBuilder createLedger();
}
class BookKeeper extends IBookKeeper {
ICreateBuilder createLedger() {
// return a ledger create op builder
}
}
there are a few benefits using this approach:
- this approach can be used for other operations - create/delete/open. we
will have a consistent style for different ops and easy to extend in future.
- we can use this approach to cleanup all the interfaces. so we can
separate interface from implementation eventually, without breaking
existing API. then we can encourage people to use new API and phase out the
old API.
- we should also have interface for LedgerHandle, try to separate write
interface from read interface to produce a cleaner interface.
Any thoughts?
- Sijie
On Fri, Sep 1, 2017 at 1:33 AM, Enrico Olivelli <eolive...@gmail.com> wrote:
> Hi all,
> I have just posted a proposal to introduce a new createLedger API using the
> 'builder' design pattern.
>
> https://cwiki.apache.org/confluence/display/BOOKKEEPER/
> BP-15+New+CreateLeader+API
>
> This is a pre-requisite for BP-14 Relax Durability and for LedgerType
> improvements
>
> It is a trivial change but it will be the base for future enhancements
>
> I will send a patch as soon as we agree on the proposal
>
> Cheers
> Enrico Olivelli
>
