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


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


trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSMessagingUtils.java
<https://reviews.apache.org/r/9622/#comment39631>

    replace colon with fullstop



trunk/client/src/main/java/org/apache/oozie/client/event/jms/MessageDeserializer.java
<https://reviews.apache.org/r/9622/#comment39632>

    rename variables wfJobStartMsg and caWaitingMsg to wfJobMsg and cActionMsg 
respectively..or similar



trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java
<https://reviews.apache.org/r/9622/#comment39633>

    thanks!



trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java
<https://reviews.apache.org/r/9622/#comment39635>

    what happens as a consequence of this exception thrown?



trunk/core/src/test/java/org/apache/oozie/jms/TestJMSJobEventListener.java
<https://reviews.apache.org/r/9622/#comment39636>

    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



trunk/core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java
<https://reviews.apache.org/r/9622/#comment39637>

    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


- Mona Chitnis


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

Reply via email to