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 <f...@yahoo-inc.com> 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 > > f...@yahoo-inc.com > direct +34 93-183-8828 > > avinguda diagonal 177, 8th floor, barcelona, 08018, es > phone (408) 349 3300 fax (408) 349 3301 > >
