You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org> on 2011/11/03 14:37:34 UTC
[jira] [Commented] (BOOKKEEPER-90) Hedwig API changes for initial
Bookkeeper release
[ https://issues.apache.org/jira/browse/BOOKKEEPER-90?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143164#comment-13143164 ]
jiraposter@reviews.apache.org commented on BOOKKEEPER-90:
---------------------------------------------------------
-----------------------------------------------------------
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:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2546/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-25 08:50:06)
bq.
bq.
bq. Review request for bookkeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. HedwigClient#getSslFactory shouldn't be public
bq. HedwigClient#getConsumeCallback shouldn't be public
bq. HedwigClient#doConnect shouldn't be public
bq. HedwigClient#getHostFromChannel shouldn't be public
bq. HedwigClient#getResponseHandlerFromChannel shouldn't be public
bq. HedwigClient#getHostForTopic shouldn't be public
bq. HedwigClient#clearAllTopicsForHost shouldn't be public
bq. HedwigClient#getClientTimer shoulnd't be public
bq. HedwigClient#stop should throw some sort of Exception in the case of errors
bq.
bq. HedwigPublisher#publish shouldn't use protobuf ByteString, as it requires the user to import protobufs
bq. HedwigPublisher#getChannelForHost shouldn't be public
bq.
bq. HedwigSubscriber#HedwigSubscriber shouldn't be public
bq. HedwigSubscriber#doConsume shouldn't be public
bq. HedwigSubscriber#hasSubscription probably shouldn't be public
bq. HedwigSubscriber#getSubscriptionList shoulnd't exist
bq. HedwigSubscriber#getChannelForTopic shouldn't be public
bq. HedwigSubscriber#setChannelforTopic shouldn't be public
bq. HedwigSubscriber#removeChannelForTopic shound't be public
bq.
bq. MessageHandler#consume should be called 'deliver'
bq.
bq. 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.
bq.
bq.
bq. This addresses bug BOOKKEEPER-90.
bq. https://issues.apache.org/jira/browse/BOOKKEEPER-90
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. hedwig-client/src/main/java/org/apache/hedwig/client/HedwigClient.java PRE-CREATION
bq. hedwig-client/src/main/java/org/apache/hedwig/client/api/Client.java PRE-CREATION
bq. hedwig-client/src/main/java/org/apache/hedwig/client/api/MessageHandler.java ddf92b1
bq. hedwig-client/src/main/java/org/apache/hedwig/client/benchmark/BenchmarkPublisher.java 687062b
bq. hedwig-client/src/main/java/org/apache/hedwig/client/benchmark/BenchmarkSubscriber.java 0e87dd7
bq. hedwig-client/src/main/java/org/apache/hedwig/client/benchmark/HedwigBenchmark.java 643f6d9
bq. hedwig-client/src/main/java/org/apache/hedwig/client/handlers/MessageConsumeCallback.java 2e11d63
bq. hedwig-client/src/main/java/org/apache/hedwig/client/handlers/PublishResponseHandler.java 90e62ba
bq. hedwig-client/src/main/java/org/apache/hedwig/client/handlers/SubscribeReconnectCallback.java 60388fa
bq. hedwig-client/src/main/java/org/apache/hedwig/client/handlers/SubscribeResponseHandler.java 2256a68
bq. hedwig-client/src/main/java/org/apache/hedwig/client/handlers/UnsubscribeResponseHandler.java f12e476
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/ClientChannelPipelineFactory.java 9c36e87
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/ConnectCallback.java 3f13754
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClient.java b270d34
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClientImpl.java PRE-CREATION
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigPublisher.java e381107
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java 7b21a4d
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/ResponseHandler.java 378bb19
bq. hedwig-client/src/main/java/org/apache/hedwig/client/netty/WriteCallback.java a8cac77
bq. hedwig-server/src/main/java/org/apache/hedwig/server/proxy/HedwigProxy.java f8b836a
bq. hedwig-server/src/main/java/org/apache/hedwig/server/proxy/ProxyStartDeliveryHandler.java 07706c1
bq. hedwig-server/src/main/java/org/apache/hedwig/server/regions/HedwigHubClient.java 0289c16
bq. hedwig-server/src/main/java/org/apache/hedwig/server/regions/HedwigHubSubscriber.java 4e8a405
bq. hedwig-server/src/main/java/org/apache/hedwig/server/regions/RegionManager.java 9533d8c
bq. hedwig-server/src/test/java/org/apache/hedwig/client/TestPubSubClient.java 64a851f
bq. hedwig-server/src/test/java/org/apache/hedwig/server/HedwigRegionTestBase.java d320c6b
bq. hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigHub.java 8ccb359
bq. hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigRegion.java 3a010c2
bq. hedwig-server/src/test/java/org/apache/hedwig/server/netty/TestPubSubServer.java 822f442
bq. pom.xml 2392db5
bq.
bq. Diff: https://reviews.apache.org/r/2546/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Ivan
bq.
bq.
> 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
> Assignee: Ivan Kelly
> Fix For: 4.0.0
>
> Attachments: BOOKKEEPER-90.diff, 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