----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9602/#review18570 -----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java <https://reviews.apache.org/r/9602/#comment38925> will it be exposed to user? if yes, than it is better to define it on the client package. trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java <https://reviews.apache.org/r/9602/#comment38926> remove those code , if not needed in future. trunk/core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java <https://reviews.apache.org/r/9602/#comment38927> How the user will know if it is DOENWITH ERROR or KEILLED? Any message text associated with that could be helpful. trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment38928> should be at least WARN. trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment38929> WARN. better error handling. please check how it is handled in CallableQueuService trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment38930> use the pattern like else { break }. Also a LOG info/debug to define why it reached here. trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment38931> Do we need to test this always? If possible, make sure it will be always instantiated in class level. trunk/core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java <https://reviews.apache.org/r/9602/#comment38932> Do we need to handle START_MANUAL or END_MANUAL? Because user needs to know they need to intervene. trunk/core/src/main/java/org/apache/oozie/event/listener/DummyJobEventListener.java <https://reviews.apache.org/r/9602/#comment38933> Why do we need it? for unit-testing? trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38935> Very long method. If possible, modularize into multiple method. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38936> is it possible, 'listernerclass' could be 'null'. in that case add "listernerClass != null && i <linsternerClass.lentgth) trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38937> Is this a right approach? Just report the failure. shoudl it fail the whole service? I'm sure also. Do you? trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38938> what does it mean? WF_JOB and COORD_ACTION is must? If someone wants *only* COORD_JOB. How will it be achieved? trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38939> so WFJOB could be twice. right? one from default and another from config. Please rethink about this behavior. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38940> Default '0' is correct? trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38941> could be write how many threads are determine finally. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38942> For better modularization, make it a separate method something like runJob() and the else part as runSLA() trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38943> Add the jobType in the message trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38944> Same trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38945> same... - Mohammad Islam On March 31, 2013, 6:07 p.m., Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9602/ > ----------------------------------------------------------- > > (Updated March 31, 2013, 6:07 p.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 1462882 > 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 1462882 > > trunk/core/src/main/java/org/apache/oozie/client/rest/JsonCoordinatorAction.java > 1462882 > trunk/core/src/main/java/org/apache/oozie/command/XCommand.java 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionReadyXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordKillXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordinatorXCommand.java > 1462882 > > trunk/core/src/main/java/org/apache/oozie/command/wf/ActionCheckXCommand.java > 1462882 > trunk/core/src/main/java/org/apache/oozie/command/wf/KillXCommand.java > 1462882 > trunk/core/src/main/java/org/apache/oozie/command/wf/ResumeXCommand.java > 1462882 > trunk/core/src/main/java/org/apache/oozie/command/wf/SignalXCommand.java > 1462882 > trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java > 1462882 > trunk/core/src/main/java/org/apache/oozie/command/wf/WorkflowXCommand.java > 1462882 > 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/DummyJobEventListener.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 > 1462882 > > trunk/core/src/main/java/org/apache/oozie/sla/event/listener/SLAEventListener.java > PRE-CREATION > trunk/core/src/main/resources/oozie-default.xml 1462882 > > trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java > 1462882 > 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 > >
