qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "rajith attapattu" <rajit...@gmail.com>
Subject Re: Review Request: Messenger API and implementation
Date Wed, 12 Dec 2012 03:39:47 GMT

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


1. It would be good to have Java doc for the API classes.

2. As you suggest a factory approach might be good especially given that Phil and Keith are
working on a version on top of the C impl via swig.

3. Given above, it might be better to have Message.java as an interface and move the current
impl into a MessageImpl.java under the impl directory.


/proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/impl/MessengerImpl.java
<https://reviews.apache.org/r/7934/#comment30543>

    Perhaps allows this to be configured via a system prop?
    
    (not really an issue bcos you auto-expand)



/proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/impl/MessengerImpl.java
<https://reviews.apache.org/r/7934/#comment30542>

    Should we not close the connector here?
    It looks like we are leaking tcp connections here ?



/proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/impl/MessengerImpl.java
<https://reviews.apache.org/r/7934/#comment30546>

    Perhaps we should have a read and write buffer with a sensible default and then allow
it to be configured via a system prop.
    
    This way it allows an application to provide a reasonable size as it *may* know the message
sizes they are expecting.


- rajith attapattu


On Dec. 10, 2012, 10:29 p.m., Gordon Sim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7934/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2012, 10:29 p.m.)
> 
> 
> Review request for qpid, rajith attapattu, Rafael Schloming, and Rob Godfrey.
> 
> 
> Description
> -------
> 
> Added Messenger API and impl. Probably need some sort of factory if we keep this split
to avoid direct construction of the impl class.
> 
> 
> This addresses bug PROTON-118.
>     https://issues.apache.org/jira/browse/PROTON-118
> 
> 
> Diffs
> -----
> 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/AcceptMode.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/Messenger.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/MessengerException.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/Status.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/Tracker.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/impl/MessengerImpl.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/impl/TrackerImpl.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/messenger/impl/TrackerQueue.java
PRE-CREATION 
>   /proton/trunk/proton-j/proton/src/main/scripts/proton.py 1419839 
> 
> Diff: https://reviews.apache.org/r/7934/diff/
> 
> 
> Testing
> -------
> 
> I have a little bit more work required on the test shim to handle accessing message body
as string in python tests (currently hacked around that to get tests passing). Also the 100K
and 1M message tests don't pass yet (not sure where the issue is for that).
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message