----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/676/#review616 -----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java <https://reviews.apache.org/r/676/#comment1264> There are several tabs introduced by the patch here, they should be changed to spaces. Should we really replace the IllegalStateException already being thrown previously? The previous behaviour was to chain the additional exception to it. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java <https://reviews.apache.org/r/676/#comment1263> If closed() were used for the task instead of this method then it could remain private. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java <https://reviews.apache.org/r/676/#comment1262> There is a tab introduced by the patch, it should be changed to spaces. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java <https://reviews.apache.org/r/676/#comment1260> There are several tabs introduced by the patch, they should be changed to spaces. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java <https://reviews.apache.org/r/676/#comment1259> Should we not be using AMQSession.closed() as the super implementation, rather than the close() method? http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_8.java <https://reviews.apache.org/r/676/#comment1261> There are several tabs introduced by the patch here, they should be changed to spaces. The code would probably look cleaner and potentially be more accurate by calling manager.getLastException() once upfront and using the result for all further processing rather than calling the method up to 3 times. - Robbie On 2011-04-29 03:15:23, rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/676/ > ----------------------------------------------------------- > > (Updated 2011-04-29 03:15:23) > > > Review request for qpid. > > > Summary > ------- > > Any execution exception received will now be notified to AMQSession_0_10 via > the SessionListener. > Also the AMQSession_0_10 will be marked closed. > > The Exception thrown when the session is accessed after it was closed, will > now contain information about the underlying AMQ Exception that caused the > session closure. > > > This addresses bug QPID-3233. > https://issues.apache.org/jira/browse/QPID-3233 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java > 1097636 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java > 1097636 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_8.java > 1097636 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/message/TestAMQSession.java > 1097636 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java > 1097636 > > Diff: https://reviews.apache.org/r/676/diff > > > Testing > ------- > > > Thanks, > > rajith > >
