> On Sept. 4, 2013, 8:54 p.m., Robert Kanter wrote: > > Did you add any tests with DST? DST can get really tricky so its important > > that we test it. > > > > Also, OOZIE-1449 is, among other things, consolidating the JPA executor > > classes so we don't have so many of them; you should probably coordinate > > with Ryota to make sure your patches don't conflict with the JPA stuff. > > Bowen Zhang wrote: > So, my question is since we are writing cron syntax in UTC time/oozie > processing timezone, then what's the problem with DST since UTC doesn't have > DST? If we really want to test DST, we need to create a public function in > DateUtil to set oozie processing timezone to PST only for testing purpose. Is > this what we are going to do? > For JPA executor, my patch doesn't touch JPA executors at all. > > Rohini Palaniswamy wrote: > Even though time is in UTC, DST transitions will have an effect if user > uses a different timezone in the coordinator. > > > http://oozie.apache.org/docs/3.1.3-incubating/CoordinatorFunctionalSpec.html > While Oozie coordinator engine works in UTC, it provides DST support for > coordinator applications. > > And please do add/update test cases that validate nominal time for coord > actions materialized for regular (not cron) frequencies also, because the > code change impacts that too. Emphasizing heavily on unit tests because the > existing tests do not fully cover materialization. They only check for number > of actions materialized and not their actual nominal times. If the > materialization is wrong, then it will be a big issue.
For the JPA executor, I just meant that in the patch, there are places where you execute a JPA command. The way this is done will change with OOZIE-1499, for example: - jpaService.execute(new CoordActionUpdateForInputCheckJPAExecutor(action)); + CoordActionQueryExecutor.getInstance().executeUpdate(CoordActionQuery.UPDATE_COORD_ACTION_FOR_INPUTCHECK, action); So you'll need to make minor changes if your patch goes in after OOZIE-1499 or Ryota will need to make some changes to update this patches use of jpaService.execute(...) if it goes in before OOZIE-1499. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13362/#review25903 ----------------------------------------------------------- On Aug. 30, 2013, 10:54 p.m., Bowen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13362/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2013, 10:54 p.m.) > > > Review request for oozie, Robert Kanter and Rohini Palaniswamy. > > > Repository: oozie > > > Description > ------- > > oozie-1306 cron scheduling for coordinator job > > > Diffs > ----- > > > /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java > 1518847 > > /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1518847 > > /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java > 1518847 > > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java > 1518847 > > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java > 1518847 > /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION > /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION > /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/13362/diff/ > > > Testing > ------- > > > Thanks, > > Bowen Zhang > >
