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

Flavio Junqueira commented on BOOKKEEPER-311:
---------------------------------------------

In general the patch looks good to me, but I have a few minor comments:

- There is one TODO in the patch. For every TODO, it would be best to have a 
jira created. If the TODO is a blocker, the jira needs to be marked as such.
- This patch seems to introduce a discrepancy between the api of the java and 
of the C clients. If not fixed in this patch, which I'm fine with no doing, 
then we need to create a jira a mark the jira as a blocker.
- FORMAT: There are some long lines in this patch, in particular the javadoc 
for asyncPublishWithResponse. 
 
                
> Changes in hedwig client api to support JMS spec
> ------------------------------------------------
>
>                 Key: BOOKKEEPER-311
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-311
>             Project: Bookkeeper
>          Issue Type: Sub-task
>            Reporter: Mridul Muralidharan
>         Attachments: hedwig-client-consume.patch.1, 
> hedwig-client-publish.patch.1, hedwig-client.patch, hedwig-client.patch.3, 
> hedwig-client.patch.4
>
>
> Primary changes are :
> a) Add support for returning seq-id for a publish request. This is an api 
> change (backwardly compatible for users).
> b) Make consume a sync consume, with addition of an asyncConsume - this is to 
> ensure that invoking consume() ensure request makes to server before 
> returning (with what reasonable gaurantees that netty allows).
> c) Ensure that explicit close'ing of session will flush buffered consume 
> seq-id's when auto-ack is enabled (default in hedwig java client).
> In addition, there are also fixes for
> d) Fix NPE's observed as part of testing JMS provider.

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