Yes, I agree on the 4.0 release thing. It will make it clear that there's been a big change.
I've created two JIRAs for the changes, BOOKKEEPER-89 for bookkeeper, BOOKKEEPER-90 for hedwig. Please have a look [1][2]. BOOKKEEPER-89 is complete. For BOOKKEEPER-90, I still need to decide what to do about ByteString if we do anything at all. Having ByteString in the API tries us to google protobufs. Im not sure this is a huge issue, but it means that this API will never be able to move away from it. -Ivan [1] https://reviews.apache.org/r/2543/ [2] https://reviews.apache.org/r/2546/ On 22 Oct 2011, at 00:27, Benjamin Reed wrote: > i also agee. we should also call it a 4.0 release. > > ben > > On Fri, Oct 21, 2011 at 2:17 AM, Ivan Kelly <[email protected]> wrote: >> I agree with Utkarsh. Initial release is as good of an excuse we'll ever >> have for breaking stuff. The changes themselves shouldn't be too big though. >> Ill have a go later today. >> >> -Ivan >> >> On 19 Oct 2011, at 19:04, Utkarsh Srivastava wrote: >> >>> I think it makes sense to delay the release if required for this >>> cleanup. Such cleanup will get only harder after the release. >>> >>> On Wed, Oct 19, 2011 at 9:42 AM, Flavio Junqueira <[email protected]> >>> wrote: >>>> Hi Ivan, Thanks for putting this list together. I don't have a good sense >>>> of >>>> how many changes the modifications you're proposing would induce. My >>>> concern >>>> is delaying the release further, even though I agree that cleaning up the >>>> interfaces is important. >>>> >>>> Also, I was thinking that some public calls are related to our package >>>> structure. In particular, I'm referring to BookKeeperTools, which is in >>>> this >>>> tools package and I've needed to make some calls public to be able to >>>> access >>>> from there. It might be a good idea to review those as well. >>>> >>>> -Flavio >>>> >>>> On Oct 19, 2011, at 5:40 PM, Ivan Kelly wrote: >>>> >>>>> I've just gone through the public apis for BK and HW and have found the >>>>> following issues. Most of them are issues with protection being too loose. >>>>> There's only 2-3 breaks here which should require code change on the >>>>> users' >>>>> part. The rest are things that they really shouldn't be using in any case. >>>>> >>>>> BookKeeper#createLedger, parameter is named passwd, "Key" used in >>>>> LedgerHandle api >>>>> BookKeeper#getBookieClient shouldn't be public >>>>> BookKeeper#createComplete shouldn't be public >>>>> BookKeeper#openComplete shouldn't be public >>>>> BookKeeper#deleteComplete shouldn't be public >>>>> BookKeeper#halt could be changed to close(), should throw a BKException >>>>> >>>>> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be >>>>> private >>>>> LedgerHandle#getLedgerMetadata shouldn't be public >>>>> LedgerHandle#getDigestManager shouldn't be public >>>>> LedgerHandle#getDistributionSchedule shouldn't be public >>>>> LedgerHandle#writeLedgerConfig shouldn't be public >>>>> LedgerHandle#addEntry should return void, errors should go in an Exception >>>>> LedgerHandle#readComplete should not be public >>>>> LedgerHandle#addComplete should not be public >>>>> LedgerHandle#readLastConfirmedCompelte should not be public >>>>> LedgerHandle#closeComplete should not be public >>>>> >>>>> ASyncCallback#RecoverCallback shouldn't be public >>>>> >>>>> HedwigClient#getSslFactory shouldn't be public >>>>> HedwigClient#getConsumeCallback shouldn't be public >>>>> HedwigClient#doConnect shouldn't be public >>>>> HedwigClient#getHostFromChannel shouldn't be public >>>>> HedwigClient#getResponseHandlerFromChannel shouldn't be public >>>>> HedwigClient#getHostForTopic shouldn't be public >>>>> HedwigClient#clearAllTopicsForHost shouldn't be public >>>>> HedwigClient#getClientTimer shoulnd't be public >>>>> HedwigClient#stop should throw some sort of Exception in the case of >>>>> errors >>>>> >>>>> HedwigPublisher#publish shouldn't use protobuf ByteString, as it requires >>>>> the user to import protobufs >>>>> HedwigPublisher#getChannelForHost shouldn't be public >>>>> >>>>> HedwigSubscriber#HedwigSubscriber shouldn't be public >>>>> HedwigSubscriber#doConsume shouldn't be public >>>>> HedwigSubscriber#hasSubscription probably shouldn't be public >>>>> HedwigSubscriber#getSubscriptionList shoulnd't exist >>>>> HedwigSubscriber#getChannelForTopic shouldn't be public >>>>> HedwigSubscriber#setChannelforTopic shouldn't be public >>>>> HedwigSubscriber#removeChannelForTopic shound't be public >>>>> >>>>> MessageHandler#consume should be called 'deliver' >>>>> >>>>> The hedwig client is under a netty package. There's nothing netty specific >>>>> about the api, so it should be in the org.apache.hedwig.client package. >>>>> >>>>> In both BK and HW, context objects are passed around with callbacks. I >>>>> don't think this is necessary, but it doesn't bug me too much. >>>>> >>>>> >>>>> -Ivan >>>> >>>> flavio >>>> junqueira >>>> >>>> research scientist >>>> >>>> [email protected] >>>> direct +34 93-183-8828 >>>> >>>> avinguda diagonal 177, 8th floor, barcelona, 08018, es >>>> phone (408) 349 3300 fax (408) 349 3301 >>>> >>>> >> >>
