[ 
https://issues.apache.org/jira/browse/BOOKKEEPER-90?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13134170#comment-13134170
 ] 

[email protected] commented on BOOKKEEPER-90:
---------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2546/
-----------------------------------------------------------

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
-----

  
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java 
6421460 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 
6af43ae 
  
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java
 PRE-CREATION 
  
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java 
d4af3fa 
  
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java
 78aaa15 
  
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 
959df73 
  
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java
 1131652 
  
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java
 94e444c 
  
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java
 dfc63d7 
  
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java
 PRE-CREATION 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 
224c796 
  
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
 82483f3 
  
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java
 56331ef 
  
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java
 f933ba1 
  
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCacheTest.java 
3a78507 
  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/benchmark/BookkeeperBenchmark.java
 a934985 
  hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java 
726341d 
  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 
  
hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java
 b918d97 
  pom.xml 2392db5 

Diff: https://reviews.apache.org/r/2546/diff


Testing
-------


Thanks,

Ivan


                
> Hedwig API changes for initial Bookkeeper release
> -------------------------------------------------
>
>                 Key: BOOKKEEPER-90
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-90
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-90.diff
>
>
> 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 message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to