> On April 12, 2013, 5:26 p.m., Mona Chitnis wrote: > > Similar to how you handle ClassNotFoundException for the > > JMSJobEventListener, should we throw exception in EventHandlerService too? > > I'm thinking if the configured listener string in invalid, then we will > > still end up filling up the event queue, with no listener draining it on > > the other end - pointless
Seems unlikely. If the configured listener string is invalid, the init() on listener will fail and EventHandlerService will not be started. Also, the messaging utils is user facing so we need to be more careful about error messages and exception handling. Probably, we need better error message and error handling for EHS but that can be added later as it only affects the system admins and not the actual users. > On April 12, 2013, 5:26 p.m., Mona Chitnis wrote: > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSMessagingUtils.java, > > line 56 > > <https://reviews.apache.org/r/9622/diff/6/?file=280508#file280508line56> > > > > replace colon with fullstop kk > On April 12, 2013, 5:26 p.m., Mona Chitnis wrote: > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/MessageDeserializer.java, > > line 58 > > <https://reviews.apache.org/r/9622/diff/6/?file=280510#file280510line58> > > > > rename variables wfJobStartMsg and caWaitingMsg to wfJobMsg and > > cActionMsg respectively..or similar kk > On April 12, 2013, 5:26 p.m., Mona Chitnis wrote: > > trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java, > > line 140 > > <https://reviews.apache.org/r/9622/diff/6/?file=280524#file280524line140> > > > > what happens as a consequence of this exception thrown? A runtime exception will be thrown if it comes on this block. It might not happen bcoz currently events are generated only for workflow job and coordinator action. So this method wont be called. However, I will change this to warning instead of exception as the caller thread might be executing a batch of events and we dont want it to die if one of the event fails. > On April 12, 2013, 5:26 p.m., Mona Chitnis wrote: > > trunk/core/src/test/java/org/apache/oozie/jms/TestJMSJobEventListener.java, > > line 292 > > <https://reviews.apache.org/r/9622/diff/6/?file=280527#file280527line292> > > > > can another test be added to check a combination EventStatus=abc AND > > MessageType=JOB, and given that both Workflowjob and coordinatorAction > > events were produced, both should be received We have selector on five fields, and we think about AND or OR on all of them, the no. of combintations will be huge. So I am only having a a basic AND and OR test on selectors. Also, the selectors are applied on JMS side. So even if the unit test case works bcoz it uses activemq, there is no gaurantee that the integration test will go through as the JMS provider may be different. So we can have more combination of selectors and events in end to end test. > On April 12, 2013, 5:26 p.m., Mona Chitnis wrote: > > trunk/core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java, > > line 145 > > <https://reviews.apache.org/r/9622/diff/6/?file=280528#file280528line145> > > > > please edit this line to have "Dummy workflow job " + > > wje.getEventStatus() and assert statement in the test should assert for the > > status too, like before kk - Virag ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9622/#review19083 ----------------------------------------------------------- On April 11, 2013, 7:49 p.m., Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9622/ > ----------------------------------------------------------- > > (Updated April 11, 2013, 7:49 p.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 1467063 > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSHeaderConstants.java > PRE-CREATION > > 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/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/message/CoordinatorActionMessage.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/message/EventMessage.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/message/JobMessage.java > PRE-CREATION > > trunk/client/src/main/java/org/apache/oozie/client/event/message/WorkflowJobMessage.java > PRE-CREATION > trunk/core/pom.xml 1467063 > trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java > 1467063 > > trunk/core/src/main/java/org/apache/oozie/event/listener/JobEventListener.java > 1467063 > > 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 > 1467063 > trunk/core/src/main/java/org/apache/oozie/jms/DefaultConnectionContext.java > 1467063 > trunk/core/src/main/java/org/apache/oozie/jms/JMSExceptionListener.java > 1467063 > trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java > 1467063 > > 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/TestEventHandlerService.java > 1467063 > > trunk/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java > 1467063 > trunk/pom.xml 1467063 > > 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 > >
