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