----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17750/#review33957 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronModule.java <https://reviews.apache.org/r/17750/#comment63789> Should it rather take "aurora-cron" and UUID.randomUUID() here? I am afraid any messages coming from quartz scheduler would duplicate the long name. src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronPredictor.java <https://reviews.apache.org/r/17750/#comment63796> s/public// src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java <https://reviews.apache.org/r/17750/#comment63797> Mixing fields with methods. src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java <https://reviews.apache.org/r/17750/#comment63799> Please, move into constructor for better consistency and visibility. src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java <https://reviews.apache.org/r/17750/#comment63802> s/@Override/@Override\n/g src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java <https://reviews.apache.org/r/17750/#comment63803> + scheduler.getSchedulerName()? src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java <https://reviews.apache.org/r/17750/#comment63807> String.format() seems to be used only here. All other places use concatenation. Converge on one approach? Also, e should be on a separate line. src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java <https://reviews.apache.org/r/17750/#comment63804> Move this outside try..catch? src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronIT.java <https://reviews.apache.org/r/17750/#comment63809> Drop. src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronPredictorTest.java <https://reviews.apache.org/r/17750/#comment63810> remove src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronSchedulerTest.java <https://reviews.apache.org/r/17750/#comment63811> Assert.fail()? src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronSchedulerTest.java <https://reviews.apache.org/r/17750/#comment63813> Same. - Maxim Khutornenko On Feb. 5, 2014, 11:23 p.m., Kevin Sweeney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17750/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2014, 11:23 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-132 > https://issues.apache.org/jira/browse/AURORA-132 > > > Repository: aurora > > > Description > ------- > > CronScheduler implementation based on Quartz. > > This adds an implementation of CronScheduler using Quartz [1], allowing > Aurora to execute cron jobs without external triggering. The quartz library > is Apache 2 licensed and thus including it in Aurora complies with the ASF's > third-party licensing policy. > > [1] http://quartz-scheduler.org > > TODO (Subsequent reviews): > * Drop .noop package; make QuartCronModule the default > * Delete .testing package; merge QuartzCronIT and AbstractCronIT > * Delete thrift testing fixtures. > * Add examples of cron jobs to examples/jobs > * Add e2e test coverage for new examples > * Document cron schedule syntax in config reference. > * Create epic for reliable cron execution. > > > Diffs > ----- > > build.gradle 5b98659b9cb93a25f0f196285910cf6afb416072 > src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronModule.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronPredictor.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronScheduler.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronIT.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronPredictorTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzCronSchedulerTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/17750/diff/ > > > Testing > ------- > > ./gradlew build > > > Thanks, > > Kevin Sweeney > >
