> On April 11, 2013, 9:13 a.m., Ryota Egashira wrote: > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSMessagingUtils.java, > > line 40 > > <https://reviews.apache.org/r/9622/diff/5/?file=280095#file280095line40> > > > > you can use ParamChecker
Paramchecker is in core package. So, to use it I have to move it to client. So I think its better we dont use it. > On April 11, 2013, 9:13 a.m., Ryota Egashira wrote: > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/JMSMessagingUtils.java, > > line 57 > > <https://reviews.apache.org/r/9622/diff/5/?file=280095#file280095line57> > > > > doesn't it need '.' between oozie.msg.deserializer and msgFormat? good point. It needs it. > On April 11, 2013, 9:13 a.m., Ryota Egashira wrote: > > trunk/client/src/main/java/org/apache/oozie/client/event/jms/MessageDeserializer.java, > > line 52 > > <https://reviews.apache.org/r/9622/diff/5/?file=280097#file280097line52> > > > > MessageFormatException might be more specific. > > basically this getEventMessage could throw JMSException just as > > setPropertiesObject, since error in parsing message seem to be serious, > > need to be handled properly. I looked into JMS's MessageFormatException. It seems like that exception happens when any JMS client API is not used in a proper way. In this case, we are checking for whether the argument passed to getEventMessage is valid or not. Hence, illegalArgumentException seems better > On April 11, 2013, 9:13 a.m., Ryota Egashira wrote: > > trunk/client/src/main/java/org/apache/oozie/client/event/message/CoordinatorActionMessage.java, > > line 68 > > <https://reviews.apache.org/r/9622/diff/5/?file=280098#file280098line68> > > > > i forgot previous discussion, but just wonder if we still need both > > JobEvent.Status and CoordinatorAction.Status? (the same goes to workflow > > case). isn't it sufficient to just apply selector on > > CoordinatorAction.Status? > > Selector can only be applied to JobEvent.EventStatus and it is part of header. CoordinatorAction.Status will be part of message body > On April 11, 2013, 9:13 a.m., Ryota Egashira wrote: > > trunk/core/src/main/java/org/apache/oozie/event/messaging/MessageFactory.java, > > line 37 > > <https://reviews.apache.org/r/9622/diff/5/?file=280106#file280106line37> > > > > is format right? yup..that is how our formatter formats > On April 11, 2013, 9:13 a.m., Ryota Egashira wrote: > > trunk/core/src/main/java/org/apache/oozie/event/messaging/MessageFactory.java, > > line 59 > > <https://reviews.apache.org/r/9622/diff/5/?file=280106#file280106line59> > > > > MesageSerializerHolder could be null when initialization(reflection) > > fails? It wont be null, if reflection fails it will throw exception. - Virag ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9622/#review18993 ----------------------------------------------------------- On April 10, 2013, 6:38 a.m., Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9622/ > ----------------------------------------------------------- > > (Updated April 10, 2013, 6:38 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 1466337 > > 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 1466337 > trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java > 1466337 > > trunk/core/src/main/java/org/apache/oozie/event/listener/JobEventListener.java > 1466337 > > 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 > 1466337 > trunk/core/src/main/java/org/apache/oozie/jms/DefaultConnectionContext.java > 1466337 > trunk/core/src/main/java/org/apache/oozie/jms/JMSExceptionListener.java > 1466337 > trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java > 1466337 > > 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 > 1466337 > > trunk/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java > 1466337 > trunk/pom.xml 1466337 > > 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 > >
