----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9622/#review18789 -----------------------------------------------------------
Please add missing javadoc for public classes and methods trunk/client/pom.xml <https://reviews.apache.org/r/9622/#comment39216> adjust the indentation trunk/client/pom.xml <https://reviews.apache.org/r/9622/#comment39217> why not move version number management to top-level pom, and only specify dependency + scope in this client pom? I think that makes it easy to manipulate in a central place and maintains consistency across versions applied to all sub-modules trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSMessagingUtils.java <https://reviews.apache.org/r/9622/#comment39218> possibility of NPE trunk/client/src/main/java/org/apache/oozie/client/event/jms/MessageDeserializer.java <https://reviews.apache.org/r/9622/#comment39219> so this part is going to change right? since event-type is going away.. trunk/client/src/main/java/org/apache/oozie/client/event/messaging/CoordinatorActionMessage.java <https://reviews.apache.org/r/9622/#comment39223> in constructor for wf messages, the order of arguments is ...,user,appname,... Can same be maintained here too? trunk/core/src/main/java/org/apache/oozie/event/messaging/MessageFactory.java <https://reviews.apache.org/r/9622/#comment39220> is this a formatting quirk? 4 lines instead of maybe 2? Also there is an extra semicolon in the end.. trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java <https://reviews.apache.org/r/9622/#comment39221> why only this line on sysout and detail exception in log? either consolidate in one place or duplicate error message trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java <https://reviews.apache.org/r/9622/#comment39222> lines 95-100 are the same across all methods i.e. onwfsuccess, onwffailure, oncoordaction waiting etc.. it can be made into a common method - Mona Chitnis On March 28, 2013, 2:36 a.m., Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9622/ > ----------------------------------------------------------- > > (Updated March 28, 2013, 2:36 a.m.) > > > Review request for oozie. > > > Description > ------- > > Description at https://issues.apache.org/jira/browse/OOZIE-1234 > > OOZIE-1209 generates events and handles them by calling appropriate > listeners. This patch provides JMS implementation of those listeners. Also, > the messages are serialized using JSON and there is a deserializer to > construct the Java object back from json. > > > This addresses bug OOZIE-1234. > https://issues.apache.org/jira/browse/OOZIE-1234 > > > Diffs > ----- > > trunk/client/pom.xml 1459407 > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSMessagingUtils.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMS_HEADER_CONSTANTS.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JSONMessageDeserializer.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/MessageDeserializer.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/messaging/CoordinatorActionMessage.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/messaging/EventMessage.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/messaging/JobMessage.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/messaging/WorkflowJobMessage.java > PRE-CREATION > trunk/core/pom.xml 1459407 > > trunk/core/src/main/java/org/apache/oozie/event/messaging/JSONMessageSerializer.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/event/messaging/MessageFactory.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/event/messaging/MessageSerializer.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/jms/ConnectionContext.java > 1459407 > trunk/core/src/main/java/org/apache/oozie/jms/DefaultConnectionContext.java > 1459407 > trunk/core/src/main/java/org/apache/oozie/jms/JMSExceptionListener.java > 1459407 > trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/oozie/jms/TestDefaultConnectionContext.java > PRE-CREATION > trunk/core/src/test/java/org/apache/oozie/jms/TestJMSJobEventListener.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java > 1459407 > > Diff: https://reviews.apache.org/r/9622/diff/ > > > Testing > ------- > > Unit test cases added. Test case for JMS connection failure pending. End to > end test pending > > > Thanks, > > Virag Kothari > >
