You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Kevin Sweeney <ke...@apache.org> on 2014/03/27 02:37:42 UTC
Review Request 19709: CrontabEntry parser
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
Cron scheduler based on Quartz.
Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
Diffs
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java df0c37839c5da54795404c18ff9fc93084cd32e4
src/main/java/org/apache/aurora/scheduler/cron/CronScheduler.java 56e9950fd94ae1e3dbd96baec00b7e6b262fbe34
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/noop/NoopCronPredictor.java 7b25152c0258e10be21b801cae1444c518367fa7
src/main/java/org/apache/aurora/scheduler/cron/noop/NoopCronScheduler.java a31551c77818c17ee0f9f71b5ab458a3b853dc6a
src/main/java/org/apache/aurora/scheduler/cron/quartz/CronExpressions.java PRE-CREATION
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/main/java/org/apache/aurora/scheduler/cron/testing/AbstractCronIT.java 61b01d2575d6cfce069e77c39bfa8f8680cf4298
src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java e2f9ed0ea846c570de11b7dd85bc90aee6bc3342
src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java 70ccf036c0676accd4a60c91bac1e6c77f365598
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 12d33ffa0fc246d50ed95bb2399af9707087d068
src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java a9b85d0983dcfee89101a5e774ba86ee11708c68
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
src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 033ebd29a4e95914347c1f5f8ade73115585a26f
src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java 380df1011cec2970dbcf37252d469dc6373b66f6
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c89965cee38a72b23fe65cb14bfa61b4cba0bfc4
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Bill Farner <wf...@apache.org>.
> On March 27, 2014, 9:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java, line 148
> > <https://reviews.apache.org/r/19709/diff/1/?file=538165#file538165line148>
> >
> > Do these all need to be public? Looks like most of these accessors are only used within this class.
> >
> > Many others seem like they should be @VisibleTesting as well.
>
> Kevin Sweeney wrote:
> Feels weird to limit visibility for some fields just because of quartz's implementation quirks here.
Less code = better? If this were a public library i'd feel differently, but i don't see what we stand to gain here.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38799
-----------------------------------------------------------
On March 28, 2014, 12:14 a.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 28, 2014, 12:14 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-132
> https://issues.apache.org/jira/browse/AURORA-132
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Cron scheduler based on Quartz.
>
> Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
>
> I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
> src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
> On March 27, 2014, 2:44 p.m., Bill Farner wrote:
> > build.gradle, line 377
> > <https://reviews.apache.org/r/19709/diff/1/?file=538162#file538162line377>
> >
> > What would it take to do that now? Is it as simple as System.setProperty?
Dropped.
> On March 27, 2014, 2:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java, line 148
> > <https://reviews.apache.org/r/19709/diff/1/?file=538165#file538165line148>
> >
> > Do these all need to be public? Looks like most of these accessors are only used within this class.
> >
> > Many others seem like they should be @VisibleTesting as well.
Feels weird to limit visibility for some fields just because of quartz's implementation quirks here.
> On March 27, 2014, 2:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java, line 67
> > <https://reviews.apache.org/r/19709/diff/1/?file=538165#file538165line67>
> >
> > doc?
Updated.
> On March 27, 2014, 2:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java, line 71
> > <https://reviews.apache.org/r/19709/diff/1/?file=538165#file538165line71>
> >
> > doc?
Updated.
> On March 27, 2014, 2:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java, line 304
> > <https://reviews.apache.org/r/19709/diff/1/?file=538165#file538165line304>
> >
> > s/SINGLETON/INT/?
Fixed. Called it NUMBER.
> On March 27, 2014, 2:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java, line 373
> > <https://reviews.apache.org/r/19709/diff/1/?file=538165#file538165line373>
> >
> > replaceNameAliases?
Sure, fixed.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38799
-----------------------------------------------------------
On March 27, 2014, 2:52 p.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 27, 2014, 2:52 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
> -------
>
> Cron scheduler based on Quartz.
>
> Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
>
> I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
> src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38799
-----------------------------------------------------------
build.gradle
<https://reviews.apache.org/r/19709/#comment71109>
What would it take to do that now? Is it as simple as System.setProperty?
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71112>
doc?
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71113>
doc?
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71114>
Do these all need to be public? Looks like most of these accessors are only used within this class.
Many others seem like they should be @VisibleTesting as well.
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71115>
s/SINGLETON/INT/?
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71117>
replaceNameAliases?
- Bill Farner
On March 27, 2014, 1:37 a.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 27, 2014, 1:37 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-132
> https://issues.apache.org/jira/browse/AURORA-132
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Cron scheduler based on Quartz.
>
> Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
>
> I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java df0c37839c5da54795404c18ff9fc93084cd32e4
> src/main/java/org/apache/aurora/scheduler/cron/CronScheduler.java 56e9950fd94ae1e3dbd96baec00b7e6b262fbe34
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/noop/NoopCronPredictor.java 7b25152c0258e10be21b801cae1444c518367fa7
> src/main/java/org/apache/aurora/scheduler/cron/noop/NoopCronScheduler.java a31551c77818c17ee0f9f71b5ab458a3b853dc6a
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronExpressions.java PRE-CREATION
> 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/main/java/org/apache/aurora/scheduler/cron/testing/AbstractCronIT.java 61b01d2575d6cfce069e77c39bfa8f8680cf4298
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java e2f9ed0ea846c570de11b7dd85bc90aee6bc3342
> src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java 70ccf036c0676accd4a60c91bac1e6c77f365598
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 12d33ffa0fc246d50ed95bb2399af9707087d068
> src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
> src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java a9b85d0983dcfee89101a5e774ba86ee11708c68
> 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
> src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 033ebd29a4e95914347c1f5f8ade73115585a26f
> src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java 380df1011cec2970dbcf37252d469dc6373b66f6
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c89965cee38a72b23fe65cb14bfa61b4cba0bfc4
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38857
-----------------------------------------------------------
Ship it!
src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json
<https://reviews.apache.org/r/19709/#comment71184>
As discussed offline, if you use Splitter's omitEmptyStrings() you should be able to trivially support these.
- Bill Farner
On March 28, 2014, 12:14 a.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 28, 2014, 12:14 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-132
> https://issues.apache.org/jira/browse/AURORA-132
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Cron scheduler based on Quartz.
>
> Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
>
> I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
> src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 28, 2014, 10:31 a.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
crontab(5) entry parser.
Diffs
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/19709/diff/
Testing (updated)
-------
./gradlew build
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 28, 2014, 10:30 a.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Maxim's feedback.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
crontab(5) entry parser.
Diffs (updated)
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38903
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71239>
Linking to itself seems redundant here and might be confusing. Suggest using @code instead.
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java
<https://reviews.apache.org/r/19709/#comment71237>
Drop newline
- Maxim Khutornenko
On March 28, 2014, 1:02 a.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 28, 2014, 1:02 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-132
> https://issues.apache.org/jira/browse/AURORA-132
>
>
> Repository: aurora
>
>
> Description
> -------
>
> crontab(5) entry parser.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 27, 2014, 6:02 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 (updated)
-------
crontab(5) entry parser.
Diffs
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 27, 2014, 5:48 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Also fix build.gradle.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
Cron scheduler based on Quartz.
Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
Diffs (updated)
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 27, 2014, 5:46 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
More of Bill's feedback. @VisibleForTesting and workaround for schedules with trailing commas.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
Cron scheduler based on Quartz.
Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
Diffs (updated)
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 27, 2014, 5:14 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Incorporate Bill's feedback.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
Cron scheduler based on Quartz.
Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
Diffs (updated)
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
> On March 27, 2014, 2:54 p.m., Bill Farner wrote:
> > build.gradle, line 223
> > <https://reviews.apache.org/r/19709/diff/2/?file=538968#file538968line223>
> >
> > Please exclude junit-dep instead.
Done.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38821
-----------------------------------------------------------
On March 27, 2014, 2:52 p.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 27, 2014, 2:52 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
> -------
>
> Cron scheduler based on Quartz.
>
> Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
>
> I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
> src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/#review38821
-----------------------------------------------------------
build.gradle
<https://reviews.apache.org/r/19709/#comment71148>
Please exclude junit-dep instead.
- Bill Farner
On March 27, 2014, 9:52 p.m., Kevin Sweeney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19709/
> -----------------------------------------------------------
>
> (Updated March 27, 2014, 9:52 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
> -------
>
> Cron scheduler based on Quartz.
>
> Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
>
> I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
>
>
> Diffs
> -----
>
> build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
> src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
>
> Diff: https://reviews.apache.org/r/19709/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
> Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
>
>
> Thanks,
>
> Kevin Sweeney
>
>
Re: Review Request 19709: CrontabEntry parser
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19709/
-----------------------------------------------------------
(Updated March 27, 2014, 2:52 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Minimize diff.
Bugs: AURORA-132
https://issues.apache.org/jira/browse/AURORA-132
Repository: aurora
Description
-------
Cron scheduler based on Quartz.
Apologies for the monster diff - I will try to split it but wanted to give y'all a chance to take a look in the evening/morning.
I'd suggest reviewing CrontabEntry and CrontabEntryTest for now - the rest is plumbing.
Diffs (updated)
-----
build.gradle c2a70b0285c6afb20f6db387e50744424c572d3f
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java PRE-CREATION
src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json dced8b454c97b9c456605144161ff5c946cb384f
Diff: https://reviews.apache.org/r/19709/diff/
Testing
-------
./gradlew build
Removed a few test cases from valid-cron-schedules.json after inspection revealed they should be considered invalid.
Thanks,
Kevin Sweeney