----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35724/#review98751 -----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/EntityUtil.java (line 257) <https://reviews.apache.org/r/35724/#comment155597> This is incorrect, reason being that for certain frequencies like monthly the time in millis between each run keeps changing. This will fail e.g. for monthly feeds. scheduler/pom.xml (line 92) <https://reviews.apache.org/r/35724/#comment155332> Any reasons to not use 2.2.1 which is the latest? scheduler/pom.xml (line 103) <https://reviews.apache.org/r/35724/#comment155333> 1.10.19 is the latest. scheduler/pom.xml (line 109) <https://reviews.apache.org/r/35724/#comment155336> why not 2.8.2? scheduler/src/main/java/org/apache/falcon/exception/DAGEngineException.java (line 23) <https://reviews.apache.org/r/35724/#comment155337> Description seems to be incorrect. scheduler/src/main/java/org/apache/falcon/notification/service/FalconNotificationService.java (line 48) <https://reviews.apache.org/r/35724/#comment155606> Incorrect javadoc, it is actually for registering. scheduler/src/main/java/org/apache/falcon/notification/service/event/DataEvent.java (line 33) <https://reviews.apache.org/r/35724/#comment155602> numInstances and it's getter setters are not being used. Please remove it. If you want for future then still delete it and file a JIRA. scheduler/src/main/java/org/apache/falcon/notification/service/event/JobCompletedEvent.java (line 28) <https://reviews.apache.org/r/35724/#comment155593> Remove stray todo statement from all classes. scheduler/src/main/java/org/apache/falcon/notification/service/event/JobCompletedEvent.java (line 37) <https://reviews.apache.org/r/35724/#comment155603> Constructor allows construction of incomplete object, which is not a good practice. scheduler/src/main/java/org/apache/falcon/notification/service/event/JobScheduledEvent.java (line 30) <https://reviews.apache.org/r/35724/#comment155604> All constructors should form complete objects, i.e. all mandatory parameters should be available after construction. Please make this change for all the Event Classes scheduler/src/main/java/org/apache/falcon/notification/service/request/DataNotificationRequest.java (line 30) <https://reviews.apache.org/r/35724/#comment155546> Remove stray TODOs scheduler/src/main/java/org/apache/falcon/notification/service/request/DataNotificationRequest.java (line 37) <https://reviews.apache.org/r/35724/#comment155554> This field and it's getters and setters are unused. Please remove it. scheduler/src/main/java/org/apache/falcon/notification/service/request/DataNotificationRequest.java (line 43) <https://reviews.apache.org/r/35724/#comment155548> not used. Even if it is for future use cases then please remove it for now and file a JIRA so that it doesn't get dropped. scheduler/src/main/java/org/apache/falcon/notification/service/request/NotificationRequest.java (line 38) <https://reviews.apache.org/r/35724/#comment155608> ORDER is not being set in any of the subclasses. scheduler/src/main/java/org/apache/falcon/notification/service/request/TimeNotificationRequest.java (line 31) <https://reviews.apache.org/r/35724/#comment155609> Builder pattern should not be used for supplying mandatory patterns as it allows creation of incomplete objects. It should be used only for optional parameters. Please change this from all NotificationRequest subclasses. scheduler/src/main/java/org/apache/falcon/state/EntityState.java (line 71) <https://reviews.apache.org/r/35724/#comment155599> Incorrect message. scheduler/src/main/java/org/apache/falcon/state/ID.java (line 33) <https://reviews.apache.org/r/35724/#comment155590> ID is overloaded for both entity key and instance key. Both should have separate classes as the mandatory parameters for both of them are different. scheduler/src/main/java/org/apache/falcon/state/ID.java (line 145) <https://reviews.apache.org/r/35724/#comment155592> toString seems to be used for InstanceKey this should be separated out. scheduler/src/main/java/org/apache/falcon/state/ID.java (line 189) <https://reviews.apache.org/r/35724/#comment155589> This is incorrect. compareTo should be called only on objects of same/comparable type and not on generic object. If you use the parameterized version of " Comparator<ID>" this will give you compile time error. - Ajay Yadava On July 28, 2015, 11:07 a.m., Pallavi Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35724/ > ----------------------------------------------------------- > > (Updated July 28, 2015, 11:07 a.m.) > > > Review request for Falcon and Srikanth Sundarrajan. > > > Bugs: FALCON-1213 > https://issues.apache.org/jira/browse/FALCON-1213 > > > Repository: falcon-git > > > Description > ------- > > The patch has the basic framework for the scheduler. Each of the individual > service needs to be implemented completely and will be done as separate > JIRAs. The intention of the patch is ensure the base framework satisfies all > use cases and get any early feedback in terms of course correction. > > > Diffs > ----- > > common/pom.xml 36de1f5 > common/src/main/java/org/apache/falcon/entity/EntityUtil.java b86d9d7 > > common/src/main/java/org/apache/falcon/workflow/WorkflowJobEndNotificationService.java > c4f8843 > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java cfdc84d > pom.xml 31997e8 > scheduler/pom.xml PRE-CREATION > scheduler/src/main/java/org/apache/falcon/exception/DAGEngineException.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/exception/InvalidStateTransitionException.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/exception/ServiceException.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/exception/StateStoreException.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/execution/EntityExecutor.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/execution/FalconExecutionService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/execution/NotificationHandler.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/FalconNotificationService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/ServicesRegistry.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/DataEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/Event.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/JobCompletedEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/JobScheduledEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/TimeElapsedEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/DataNotificationService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/JobCompletionService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/TimeNotificationService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/DataNotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/JobCompletionNotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/JobScheduleNotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/NotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/TimeNotificationRequest.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/predicate/Predicate.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/EntityState.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/EntityStateChangeHandler.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/ID.java PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/InstanceState.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/InstanceStateChangeHandler.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/StateMachine.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/StateService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/AbstractStateStore.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/store/StateStore.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/workflow/engine/DAGEngine.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/workflow/engine/OozieDAGEngine.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/notification/service/SchedulerServiceTest.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/notification/service/TimeNotificationServiceTest.java > PRE-CREATION > scheduler/src/test/java/org/apache/falcon/predicate/PredicateTest.java > PRE-CREATION > scheduler/src/test/java/org/apache/falcon/state/EntityStateServiceTest.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/state/InstanceStateServiceTest.java > PRE-CREATION > scheduler/src/test/resources/config/cluster/cluster-0.1.xml PRE-CREATION > scheduler/src/test/resources/config/feed/feed-0.1.xml PRE-CREATION > scheduler/src/test/resources/config/process/process-0.1.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/35724/diff/ > > > Testing > ------- > > New UTs have been added. > > > Thanks, > > Pallavi Rao > >
