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
