You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mark Chu-Carroll <mc...@twopensource.com> on 2014/05/13 17:56:05 UTC

Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

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

Review request for Aurora, David McLaughlin and Bill Farner.


Bugs: aurora-417
    https://issues.apache.org/jira/browse/aurora-417


Repository: aurora


Description
-------

Add cron schedule and deschedule calls to the scheduler API.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
  src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java f33059904fb5f6fc18a3a66dfe63c059008affe3 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9bb5c255e1118038b953fc72f0a074d461ba1fb5 
  src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
  src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java da6c0ff1bdfaefef2f72ad2559f4059cd44612b6 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 

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


Testing
-------

Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)


Thanks,

Mark Chu-Carroll


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/#review43388
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
<https://reviews.apache.org/r/21383/#comment77474>

    Kevin and i created new procedure just last week to keep track of feature deprecations.
    
    Summary:
      - Create a ticket under epic AURORA-423
      - LOG.warning when deprecated behavior is observed
      - Add a self-assigned TODO linking to your deprecation ticket



src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
<https://reviews.apache.org/r/21383/#comment77470>

    empty line above.  idea is that the line padding helps the method signature stand out more.
    
      public void longMethodSignature(..)
          throws SomeExceptionType {
      
        // Body starts here.
      }



src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
<https://reviews.apache.org/r/21383/#comment77475>

    TODO: merge CronJobManager createJob/updateJob



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77484>

    Please add test coverage for these new methods.



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77471>

    ditto - empty line above



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77476>

    indenting is off, here and in the catch block



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77477>

    This should have a Lock guard as well.



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77478>

    revert



src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/21383/#comment77480>

    > The request will be denied if the job is already present in the schedule
    
    Is this true?



src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/21383/#comment77481>

    Wording nit on the second sentence.  How about:
    
    "The request will fail if the job has not been previously scheduled through scheduleCronJob."



src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
<https://reviews.apache.org/r/21383/#comment77482>

    How about a test for deschedule failing?



src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
<https://reviews.apache.org/r/21383/#comment77479>

    remove extra newline


- Bill Farner


On May 19, 2014, 3:25 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 3:25 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/#review43535
-----------------------------------------------------------

Ship it!


lgtm.

- David McLaughlin


On May 20, 2014, 12:25 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 12:25 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/#review43538
-----------------------------------------------------------


I'd still like to see the deprecation warning and TODO in SchedulerCoreImpl so we don't forget to come back and remove cron behavior from other RPCs in 0.7.0.


src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77742>

    s/AURORA-423/AURORA-424/



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77743>

    TODO(mchucarroll)



src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
<https://reviews.apache.org/r/21383/#comment77745>

    empty line above



src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
<https://reviews.apache.org/r/21383/#comment77746>

    empty line above


- Bill Farner


On May 20, 2014, 12:25 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 12:25 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/
-----------------------------------------------------------

(Updated May 21, 2014, 9:05 a.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

Rebase to master.


Bugs: aurora-417
    https://issues.apache.org/jira/browse/aurora-417


Repository: aurora


Description
-------

Add cron schedule and deschedule calls to the scheduler API.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
  src/main/python/apache/aurora/client/cli/__init__.py 2beb9a0112459f9b1dddc64e8a7563f96f4c397f 
  src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 7bf2c327723833b547da0b09ba5ad9373a991ef4 

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


Testing
-------

Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)


Thanks,

Mark Chu-Carroll


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.

> On May 21, 2014, 2:58 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java, line 135
> > <https://reviews.apache.org/r/21383/diff/4-5/?file=584804#file584804line135>
> >
> >     remove trailing WS.
> >     
> >     this should be trivial to catch in checkstyle, i'll kick out a review shortly to do just that

Hm, it actually is caught by checkstyle.  A quick test yielded:

[ant:checkstyle] /Users/bill/code/aurora/src/main/java/org/apache/aurora/scheduler/thrift/auth/DecoratedThrift.java:16: Line has trailing spaces.


- Bill


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


On May 20, 2014, 10:56 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 10:56 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/python/apache/aurora/client/cli/__init__.py 2beb9a0112459f9b1dddc64e8a7563f96f4c397f 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.

> On May 21, 2014, 2:58 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java, line 135
> > <https://reviews.apache.org/r/21383/diff/4-5/?file=584804#file584804line135>
> >
> >     remove trailing WS.
> >     
> >     this should be trivial to catch in checkstyle, i'll kick out a review shortly to do just that
> 
> Bill Farner wrote:
>     Hm, it actually is caught by checkstyle.  A quick test yielded:
>     
>     [ant:checkstyle] /Users/bill/code/aurora/src/main/java/org/apache/aurora/scheduler/thrift/auth/DecoratedThrift.java:16: Line has trailing spaces.
> 
> Mark Chu-Carroll wrote:
>     I ran this, with tests, before submitting the updated review, and didn't get any complaints. I thought that the checkstyle tests were automatically run by gradle - in fact, I know that I got some style errors earlier on in this change. Did something change?
>     
>

Nope, nothing has changed.  Guess we'll just keep the lookout.


- Bill


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


On May 21, 2014, 1:05 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 21, 2014, 1:05 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/python/apache/aurora/client/cli/__init__.py 2beb9a0112459f9b1dddc64e8a7563f96f4c397f 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 7bf2c327723833b547da0b09ba5ad9373a991ef4 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.

> On May 20, 2014, 10:58 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java, line 135
> > <https://reviews.apache.org/r/21383/diff/4-5/?file=584804#file584804line135>
> >
> >     remove trailing WS.
> >     
> >     this should be trivial to catch in checkstyle, i'll kick out a review shortly to do just that
> 
> Bill Farner wrote:
>     Hm, it actually is caught by checkstyle.  A quick test yielded:
>     
>     [ant:checkstyle] /Users/bill/code/aurora/src/main/java/org/apache/aurora/scheduler/thrift/auth/DecoratedThrift.java:16: Line has trailing spaces.

I ran this, with tests, before submitting the updated review, and didn't get any complaints. I thought that the checkstyle tests were automatically run by gradle - in fact, I know that I got some style errors earlier on in this change. Did something change?


- Mark


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


On May 20, 2014, 6:56 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 6:56 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/python/apache/aurora/client/cli/__init__.py 2beb9a0112459f9b1dddc64e8a7563f96f4c397f 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/#review43575
-----------------------------------------------------------

Ship it!



src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
<https://reviews.apache.org/r/21383/#comment77827>

    remove trailing WS.
    
    this should be trivial to catch in checkstyle, i'll kick out a review shortly to do just that



src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
<https://reviews.apache.org/r/21383/#comment77829>

    Perfect, thanks!



src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
<https://reviews.apache.org/r/21383/#comment77828>

    +1 indent


- Bill Farner


On May 20, 2014, 10:56 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 10:56 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/python/apache/aurora/client/cli/__init__.py 2beb9a0112459f9b1dddc64e8a7563f96f4c397f 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/
-----------------------------------------------------------

(Updated May 20, 2014, 6:56 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

More review changes.


Bugs: aurora-417
    https://issues.apache.org/jira/browse/aurora-417


Repository: aurora


Description
-------

Add cron schedule and deschedule calls to the scheduler API.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
  src/main/python/apache/aurora/client/cli/__init__.py 2beb9a0112459f9b1dddc64e8a7563f96f4c397f 
  src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 

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


Testing
-------

Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)


Thanks,

Mark Chu-Carroll


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/
-----------------------------------------------------------

(Updated May 19, 2014, 8:25 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

Address Bill's review:
- Some style fixes
- Move new method bodies from CoreSchedulerImpl to SchedulerThriftInterface.
- Fix method documentation.
- Add a test of failed descheduling.


Bugs: aurora-417
    https://issues.apache.org/jira/browse/aurora-417


Repository: aurora


Description
-------

Add cron schedule and deschedule calls to the scheduler API.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
  src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2142f116afffdbd9acb63b835c240f03f417c511 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 

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


Testing
-------

Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)


Thanks,

Mark Chu-Carroll


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.

> On May 19, 2014, 9:11 p.m., Mark Chu-Carroll wrote:
> > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java, line 147
> > <https://reviews.apache.org/r/21383/diff/3/?file=584300#file584300line147>
> >
> >     Just to be clear: there is no deprecation here. The existing behavior will continue to work. We will deprecate the old cron behavior eventually, but not for a while yet.
> >     
> >     Also: is there any documented version of this procedure? I haven't seen or heard of it before, so I assume others haven't either?
> >

Is there any reason to not deprecate now?

We _literally_ made it up on Friday afternoon, so no - no docs yet.  We haven't even discussed it with anyone, so i think we're in the 'see how it works' phase.


> On May 19, 2014, 9:11 p.m., Mark Chu-Carroll wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 258
> > <https://reviews.apache.org/r/21383/diff/3/?file=584301#file584301line258>
> >
> >     There are tests of scheduleCronJob and descheduleCronJob below. Did you not notice those, or is there a particular kind of test that you're looking for?
> >

Those tests cover SchedulerCoreImpl, but not SchedulerThriftInterface.  We'll need to cover the code in both.


- Bill


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


On May 19, 2014, 3:25 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 3:25 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.

> On May 19, 2014, 9:11 p.m., Mark Chu-Carroll wrote:
> > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java, line 147
> > <https://reviews.apache.org/r/21383/diff/3/?file=584300#file584300line147>
> >
> >     Just to be clear: there is no deprecation here. The existing behavior will continue to work. We will deprecate the old cron behavior eventually, but not for a while yet.
> >     
> >     Also: is there any documented version of this procedure? I haven't seen or heard of it before, so I assume others haven't either?
> >
> 
> Bill Farner wrote:
>     Is there any reason to not deprecate now?
>     
>     We _literally_ made it up on Friday afternoon, so no - no docs yet.  We haven't even discussed it with anyone, so i think we're in the 'see how it works' phase.
> 
> Mark Chu-Carroll wrote:
>     Yes, there's a reason not to deprecate now: I don't want this to be a sudden change that disrupts all of our aurora users. I'd like to start by making a new alternative way of scheduling cron jobs, tell people, and give them some deprecation period during which to transition to the new method, before we just pull the old way out from under them.
>     
>     This change is going to have a big impact on users. I think in the long run it'll be positive, but it's the kind of thing where if it's not done gracefully, people will justifiably be very upset.

I think we want the same thing :-)

My proposal is not to remove the support, but to start the deprecation cycle now, and remove the feature in 0.7.0.


> On May 19, 2014, 9:11 p.m., Mark Chu-Carroll wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 258
> > <https://reviews.apache.org/r/21383/diff/3/?file=584301#file584301line258>
> >
> >     There are tests of scheduleCronJob and descheduleCronJob below. Did you not notice those, or is there a particular kind of test that you're looking for?
> >
> 
> Bill Farner wrote:
>     Those tests cover SchedulerCoreImpl, but not SchedulerThriftInterface.  We'll need to cover the code in both.
> 
> Maxim Khutornenko wrote:
>     Bill Farner, any reason we should continue investing into SchedulerCore? I was under assumption we were trying to get rid of it.

Thanks, that's a good point.  We've been trying to shy away from cases where SchedulerCoreImpl is just a proxy to another class.  Maxim correctly points out that SchedulerCoreImpl is just a front for CronJobManager, with little additional logic.  It's probably best to just push this code back out to SchedulerThriftInterface.


- Bill


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


On May 19, 2014, 3:25 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 3:25 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.

> On May 19, 2014, 5:11 p.m., Mark Chu-Carroll wrote:
> > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java, line 147
> > <https://reviews.apache.org/r/21383/diff/3/?file=584300#file584300line147>
> >
> >     Just to be clear: there is no deprecation here. The existing behavior will continue to work. We will deprecate the old cron behavior eventually, but not for a while yet.
> >     
> >     Also: is there any documented version of this procedure? I haven't seen or heard of it before, so I assume others haven't either?
> >
> 
> Bill Farner wrote:
>     Is there any reason to not deprecate now?
>     
>     We _literally_ made it up on Friday afternoon, so no - no docs yet.  We haven't even discussed it with anyone, so i think we're in the 'see how it works' phase.

Yes, there's a reason not to deprecate now: I don't want this to be a sudden change that disrupts all of our aurora users. I'd like to start by making a new alternative way of scheduling cron jobs, tell people, and give them some deprecation period during which to transition to the new method, before we just pull the old way out from under them.

This change is going to have a big impact on users. I think in the long run it'll be positive, but it's the kind of thing where if it's not done gracefully, people will justifiably be very upset.


- Mark


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


On May 19, 2014, 11:25 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 11:25 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On May 19, 2014, 9:11 p.m., Mark Chu-Carroll wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 258
> > <https://reviews.apache.org/r/21383/diff/3/?file=584301#file584301line258>
> >
> >     There are tests of scheduleCronJob and descheduleCronJob below. Did you not notice those, or is there a particular kind of test that you're looking for?
> >
> 
> Bill Farner wrote:
>     Those tests cover SchedulerCoreImpl, but not SchedulerThriftInterface.  We'll need to cover the code in both.

Bill Farner, any reason we should continue investing into SchedulerCore? I was under assumption we were trying to get rid of it.


- Maxim


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


On May 19, 2014, 3:25 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 3:25 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/#review43397
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
<https://reviews.apache.org/r/21383/#comment77488>

    Just to be clear: there is no deprecation here. The existing behavior will continue to work. We will deprecate the old cron behavior eventually, but not for a while yet.
    
    Also: is there any documented version of this procedure? I haven't seen or heard of it before, so I assume others haven't either?
    



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/21383/#comment77508>

    There are tests of scheduleCronJob and descheduleCronJob below. Did you not notice those, or is there a particular kind of test that you're looking for?
    


- Mark Chu-Carroll


On May 19, 2014, 11:25 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 11:25 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/
-----------------------------------------------------------

(Updated May 19, 2014, 11:25 a.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

Reworked for compatibility with the new cron code.


Bugs: aurora-417
    https://issues.apache.org/jira/browse/aurora-417


Repository: aurora


Description
-------

Add cron schedule and deschedule calls to the scheduler API.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
  src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java d377974da4f7efa7f47e06702654bc786299e01c 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4386a86c0aebdb9666c0232672f8c1bab7458f47 
  src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
  src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java 0e17f49e68953b5f043301dbb1b46b96968e1247 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 

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


Testing
-------

Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)


Thanks,

Mark Chu-Carroll


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/#review43012
-----------------------------------------------------------


Heads up: this has high likelihood of conflicting with https://reviews.apache.org/r/19767, which was reverted and needs to go back in soon.  I'm going to wager that the merge conflict is easier to deal with on this end, so i'm going to hold on this review for a day or so to give that review a chance to pick back up.

- Bill Farner


On May 14, 2014, 2:39 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21383/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 2:39 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-417
>     https://issues.apache.org/jira/browse/aurora-417
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add cron schedule and deschedule calls to the scheduler API.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java f33059904fb5f6fc18a3a66dfe63c059008affe3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9bb5c255e1118038b953fc72f0a074d461ba1fb5 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java da6c0ff1bdfaefef2f72ad2559f4059cd44612b6 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 
> 
> Diff: https://reviews.apache.org/r/21383/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21383/
-----------------------------------------------------------

(Updated May 14, 2014, 10:39 a.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

Fix python scheduler client test.


Bugs: aurora-417
    https://issues.apache.org/jira/browse/aurora-417


Repository: aurora


Description
-------

Add cron schedule and deschedule calls to the scheduler API.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 226b71ce7749492abd3e1d673382668c9011a1c8 
  src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java f33059904fb5f6fc18a3a66dfe63c059008affe3 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9bb5c255e1118038b953fc72f0a074d461ba1fb5 
  src/main/thrift/org/apache/aurora/gen/api.thrift 66292dc0369941fc62719d49209edc07adc81a53 
  src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java da6c0ff1bdfaefef2f72ad2559f4059cd44612b6 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 933a56bf3f7165fa84aedcc8d1392e32824fd487 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 399d4c614dda93ae0e7d8f38a02379bc0f42e4af 

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


Testing
-------

Ran all tests with gradlew; no errors. (Includes two new tests, that ensure that calling the new APIs causes the right internal cron methods to be called.)


Thanks,

Mark Chu-Carroll