----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2546/#review3024 -----------------------------------------------------------
most is great. thanks Ivan. it would be great not expose ByteString. but I don't find any changes about it. Is it not included in this patch? - Sijie On 2011-10-25 08:50:06, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2546/ > ----------------------------------------------------------- > > (Updated 2011-10-25 08:50:06) > > > Review request for bookkeeper. > > > Summary > ------- > > 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. > > > This addresses bug BOOKKEEPER-90. > https://issues.apache.org/jira/browse/BOOKKEEPER-90 > > > Diffs > ----- > > hedwig-client/src/main/java/org/apache/hedwig/client/HedwigClient.java > PRE-CREATION > hedwig-client/src/main/java/org/apache/hedwig/client/api/Client.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/api/MessageHandler.java > ddf92b1 > > hedwig-client/src/main/java/org/apache/hedwig/client/benchmark/BenchmarkPublisher.java > 687062b > > hedwig-client/src/main/java/org/apache/hedwig/client/benchmark/BenchmarkSubscriber.java > 0e87dd7 > > hedwig-client/src/main/java/org/apache/hedwig/client/benchmark/HedwigBenchmark.java > 643f6d9 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/MessageConsumeCallback.java > 2e11d63 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/PublishResponseHandler.java > 90e62ba > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/SubscribeReconnectCallback.java > 60388fa > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/SubscribeResponseHandler.java > 2256a68 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/UnsubscribeResponseHandler.java > f12e476 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ClientChannelPipelineFactory.java > 9c36e87 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ConnectCallback.java > 3f13754 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClient.java > b270d34 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClientImpl.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigPublisher.java > e381107 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java > 7b21a4d > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ResponseHandler.java > 378bb19 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/WriteCallback.java > a8cac77 > hedwig-server/src/main/java/org/apache/hedwig/server/proxy/HedwigProxy.java > f8b836a > > hedwig-server/src/main/java/org/apache/hedwig/server/proxy/ProxyStartDeliveryHandler.java > 07706c1 > > hedwig-server/src/main/java/org/apache/hedwig/server/regions/HedwigHubClient.java > 0289c16 > > hedwig-server/src/main/java/org/apache/hedwig/server/regions/HedwigHubSubscriber.java > 4e8a405 > > hedwig-server/src/main/java/org/apache/hedwig/server/regions/RegionManager.java > 9533d8c > hedwig-server/src/test/java/org/apache/hedwig/client/TestPubSubClient.java > 64a851f > > hedwig-server/src/test/java/org/apache/hedwig/server/HedwigRegionTestBase.java > d320c6b > > hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigHub.java > 8ccb359 > > hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigRegion.java > 3a010c2 > > hedwig-server/src/test/java/org/apache/hedwig/server/netty/TestPubSubServer.java > 822f442 > pom.xml 2392db5 > > Diff: https://reviews.apache.org/r/2546/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
