----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9602/#review18694 -----------------------------------------------------------
Very good progress. Most of them are minor comments. trunk/core/src/main/java/org/apache/oozie/command/XCommand.java <https://reviews.apache.org/r/9602/#comment39115> if not needed, please remove this. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java <https://reviews.apache.org/r/9602/#comment39117> Please use if { } Also do we need to this if? is there any possibility of null? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordinatorXCommand.java <https://reviews.apache.org/r/9602/#comment39118> Is this for "TIMEOUT" state? If not, for all other states, missing dependency is not needed, right? trunk/core/src/main/java/org/apache/oozie/event/BundleJobEvent.java <https://reviews.apache.org/r/9602/#comment39119> there are 3 statuses of type Status, jobStatus, eventStatus. The method set which status. I think it is eventStatus. In that case, the method name could be better "setEventStatus". Is this an override method? if yes, please use "@override" trunk/core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java <https://reviews.apache.org/r/9602/#comment39121> there is another status called "PREP_SUSPEND" Do we need to do anything? trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment39120> is it required? the line can be moved in after line 61. minor stuffs. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment39122> it is very long line. did u apply the formatting? trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment39124> any use of this variable (listeners)? one use could be "null" chking. if not combine line 172 & 173. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment39128> Does it need to be class level variable? what about declare it in run() {}. Also make it something more meaningful "workList". these are minor comments!!! - Mohammad Islam On April 5, 2013, 12:20 a.m., Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9602/ > ----------------------------------------------------------- > > (Updated April 5, 2013, 12:20 a.m.) > > > Review request for oozie. > > > Description > ------- > > https://issues.apache.org/jira/browse/OOZIE-1209 > WIP patch > > > This addresses bug OOZIE-1209. > https://issues.apache.org/jira/browse/OOZIE-1209 > > > Diffs > ----- > > trunk/client/src/main/java/org/apache/oozie/client/SLAEvent.java 1464795 > trunk/client/src/main/java/org/apache/oozie/client/event/Event.java > PRE-CREATION > trunk/client/src/main/java/org/apache/oozie/client/event/JobEvent.java > PRE-CREATION > trunk/client/src/main/java/org/apache/oozie/client/event/SLAEvent.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 1464795 > > trunk/core/src/main/java/org/apache/oozie/client/rest/JsonCoordinatorAction.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/command/XCommand.java 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionReadyXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordinatorXCommand.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/command/wf/KillXCommand.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/command/wf/ResumeXCommand.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/command/wf/SignalXCommand.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/command/wf/WorkflowXCommand.java > 1464795 > trunk/core/src/main/java/org/apache/oozie/event/BundleJobEvent.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/event/EventQueue.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/event/WorkflowJobEvent.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/event/listener/JobEventListener.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/service/RecoveryService.java > 1464795 > > trunk/core/src/main/java/org/apache/oozie/sla/event/listener/SLAEventListener.java > PRE-CREATION > trunk/core/src/main/resources/oozie-default.xml 1464795 > > trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java > 1464795 > trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java > PRE-CREATION > trunk/core/src/test/java/org/apache/oozie/event/TestEventQueue.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/9602/diff/ > > > Testing > ------- > > unit tests added > > > Thanks, > > Mona Chitnis > >
