Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-18 Thread Stephan Erb 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41528/#review57
---

Ship it!


Thanks for looking into this!

- Stephan Erb


On Dec. 17, 2015, 10:27 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 10:27 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread John Sirois 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41528/
---

(Updated Dec. 17, 2015, 1:34 p.m.)


Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.


Changes
---

Fix several typos.

 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java   
  | 2 +-
 
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 | 2 +-
 
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
 | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)


Bugs: AURORA-1385
https://issues.apache.org/jira/browse/AURORA-1385


Repository: aurora


Description
---

Previously,  the fact `null` could be returned by Quartz when
calculating the next run was not taken into account.  Now The
CronPredictor interface makes this possibility manifest with an
Optional result.

Code that uses the CronPredictor is adjusted and tests are added.

NB: This code is as first proposed here by Brice Arnould with small
changes: https://reviews.apache.org/r/39170/

 src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java  
  | 10 --
 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java   
  |  8 +---
 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
  | 13 -
 
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 | 23 +++
 
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
 | 35 ---
 5 files changed, 72 insertions(+), 17 deletions(-)


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
043ba7e6858db28001dfb07ea0c2ddf274a1c755 
  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
1ddc4e1946910de798f7f423dd1b19ed56dece15 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
  
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 

Diff: https://reviews.apache.org/r/41528/diff/


Testing
---

Green locally: `./gradlew -Pq build`


Thanks,

John Sirois

Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread John Sirois 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41528/
---

Review request for Aurora, Bill Farner and Zameer Manji.


Bugs: AURORA-1385
https://issues.apache.org/jira/browse/AURORA-1385


Repository: aurora


Description
---

Previously,  the fact `null` could be returned by Quartz when
calculating the next run was not taken into account.  Now The
CronPredictor interface makes this possibility manifest with an
Optional result.

Code that uses the CronPredictor is adjusted and tests are added.

NB: This code is as first proposed here by Brice Arnould with small
changes: https://reviews.apache.org/r/39170/

 src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java  
  | 10 --
 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java   
  |  8 +---
 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
  | 13 -
 
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 | 23 +++
 
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
 | 35 ---
 5 files changed, 72 insertions(+), 17 deletions(-)


Diffs
-

  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
043ba7e6858db28001dfb07ea0c2ddf274a1c755 
  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
1ddc4e1946910de798f7f423dd1b19ed56dece15 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
  
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 

Diff: https://reviews.apache.org/r/41528/diff/


Testing
---

Green locally: `./gradlew -Pq build`


Thanks,

John Sirois

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread Bill Farner 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41528/#review111037
---

Ship it!


Ship It!

- Bill Farner


On Dec. 17, 2015, 12:34 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 12:34 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread John Sirois 


> On Dec. 17, 2015, 2:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java, line 29
> > 
> >
> > nit:  is enough to split paragraphs.

Thanks - fixed.  I could have sworn I got -Xdoclint:all errors for this style 
recently over in pants land - but I just experimented and definitely am 
remembering wrong.


- John


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41528/#review111058
---


On Dec. 17, 2015, 2:27 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 2:27 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41528/#review111098
---

Ship it!


Master (93fb2c7) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Dec. 17, 2015, 9:27 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 9:27 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>