Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-03 Thread Bill Farner 

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

(Updated Sept. 3, 2014, 4:14 p.m.)


Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.


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


Repository: aurora


Description
---

The big improvement over the previous incantation of local scheduler mode is 
that arguments like `testing_isolated_scheduler` don't leak into production 
builds.  There is also less affordance made in SchedulerMain and modules for 
testing mode - the behavior changes with modules rather than branches.

This could be extended pretty easily to offer more faked behavior, but i 
stopped at providing an offer loop.  With this, jobs can be submitted, and show 
as moving to RUNNING.

As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
use the same approach.


Diffs (updated)
-

  build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
7178a924ef8d7966bf24fa96657ea514080b1d00 
  src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
PRE-CREATION 

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


Testing
---

./gradlew run, also ran directly in intellij


Thanks,

Bill Farner

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-03 Thread Bill Farner 


> On Sept. 3, 2014, 1:40 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, line 175
> > 
> >
> > Can this TODO be dropped now?

Sure, done.


- Bill


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


On Sept. 2, 2014, 11:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25257/
> ---
> 
> (Updated Sept. 2, 2014, 11:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.
> 
> 
> Bugs: AURORA-658
> https://issues.apache.org/jira/browse/AURORA-658
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The big improvement over the previous incantation of local scheduler mode is 
> that arguments like `testing_isolated_scheduler` don't leak into production 
> builds.  There is also less affordance made in SchedulerMain and modules for 
> testing mode - the behavior changes with modules rather than branches.
> 
> This could be extended pretty easily to offer more faked behavior, but i 
> stopped at providing an offer loop.  With this, jobs can be submitted, and 
> show as moving to RUNNING.
> 
> As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
> use the same approach.
> 
> 
> Diffs
> -
> 
>   build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7178a924ef8d7966bf24fa96657ea514080b1d00 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25257/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew run, also ran directly in intellij
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Kevin Sweeney 

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

Ship it!


Ship It!


src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java


Can this TODO be dropped now?



src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java


+1


- Kevin Sweeney


On Sept. 2, 2014, 4:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25257/
> ---
> 
> (Updated Sept. 2, 2014, 4:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.
> 
> 
> Bugs: AURORA-658
> https://issues.apache.org/jira/browse/AURORA-658
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The big improvement over the previous incantation of local scheduler mode is 
> that arguments like `testing_isolated_scheduler` don't leak into production 
> builds.  There is also less affordance made in SchedulerMain and modules for 
> testing mode - the behavior changes with modules rather than branches.
> 
> This could be extended pretty easily to offer more faked behavior, but i 
> stopped at providing an offer loop.  With this, jobs can be submitted, and 
> show as moving to RUNNING.
> 
> As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
> use the same approach.
> 
> 
> Diffs
> -
> 
>   build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7178a924ef8d7966bf24fa96657ea514080b1d00 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25257/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew run, also ran directly in intellij
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Zameer Manji 

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

Ship it!


LGTM

- Zameer Manji


On Sept. 2, 2014, 4:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25257/
> ---
> 
> (Updated Sept. 2, 2014, 4:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.
> 
> 
> Bugs: AURORA-658
> https://issues.apache.org/jira/browse/AURORA-658
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The big improvement over the previous incantation of local scheduler mode is 
> that arguments like `testing_isolated_scheduler` don't leak into production 
> builds.  There is also less affordance made in SchedulerMain and modules for 
> testing mode - the behavior changes with modules rather than branches.
> 
> This could be extended pretty easily to offer more faked behavior, but i 
> stopped at providing an offer loop.  With this, jobs can be submitted, and 
> show as moving to RUNNING.
> 
> As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
> use the same approach.
> 
> 
> Diffs
> -
> 
>   build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7178a924ef8d7966bf24fa96657ea514080b1d00 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25257/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew run, also ran directly in intellij
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Joshua Cohen 


> On Sept. 2, 2014, 6:31 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java, lines 
> > 134-135
> > 
> >
> > Any way to avoid magic numbers here (and would it be worthwhile)? As a 
> > n00b to the scheduler I've got no idea what these are ;).
> 
> Bill Farner wrote:
> I understand and appreciate the point, but i'm also not sure we would get 
> much out of parameterizing these.  These are arguments to 
> ScheduledExecutorService, and only dictates how frequently we will send 
> resource offers from the fake master to the scheduler.

Fair enough.


- Joshua


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


On Sept. 2, 2014, 11:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25257/
> ---
> 
> (Updated Sept. 2, 2014, 11:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.
> 
> 
> Bugs: AURORA-658
> https://issues.apache.org/jira/browse/AURORA-658
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The big improvement over the previous incantation of local scheduler mode is 
> that arguments like `testing_isolated_scheduler` don't leak into production 
> builds.  There is also less affordance made in SchedulerMain and modules for 
> testing mode - the behavior changes with modules rather than branches.
> 
> This could be extended pretty easily to offer more faked behavior, but i 
> stopped at providing an offer loop.  With this, jobs can be submitted, and 
> show as moving to RUNNING.
> 
> As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
> use the same approach.
> 
> 
> Diffs
> -
> 
>   build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7178a924ef8d7966bf24fa96657ea514080b1d00 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25257/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew run, also ran directly in intellij
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Bill Farner 

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

(Updated Sept. 2, 2014, 11:39 p.m.)


Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.


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


Repository: aurora


Description
---

The big improvement over the previous incantation of local scheduler mode is 
that arguments like `testing_isolated_scheduler` don't leak into production 
builds.  There is also less affordance made in SchedulerMain and modules for 
testing mode - the behavior changes with modules rather than branches.

This could be extended pretty easily to offer more faked behavior, but i 
stopped at providing an offer loop.  With this, jobs can be submitted, and show 
as moving to RUNNING.

As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
use the same approach.


Diffs (updated)
-

  build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
7178a924ef8d7966bf24fa96657ea514080b1d00 
  src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
 PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
PRE-CREATION 

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


Testing
---

./gradlew run, also ran directly in intellij


Thanks,

Bill Farner

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Bill Farner 


> On Sept. 2, 2014, 6:31 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java, line 184
> > 
> >
> > s/  / /

Fixed.


> On Sept. 2, 2014, 6:31 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java, lines 
> > 134-135
> > 
> >
> > Any way to avoid magic numbers here (and would it be worthwhile)? As a 
> > n00b to the scheduler I've got no idea what these are ;).

I understand and appreciate the point, but i'm also not sure we would get much 
out of parameterizing these.  These are arguments to ScheduledExecutorService, 
and only dictates how frequently we will send resource offers from the fake 
master to the scheduler.


- Bill


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


On Sept. 2, 2014, 5:16 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25257/
> ---
> 
> (Updated Sept. 2, 2014, 5:16 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.
> 
> 
> Bugs: AURORA-658
> https://issues.apache.org/jira/browse/AURORA-658
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The big improvement over the previous incantation of local scheduler mode is 
> that arguments like `testing_isolated_scheduler` don't leak into production 
> builds.  There is also less affordance made in SchedulerMain and modules for 
> testing mode - the behavior changes with modules rather than branches.
> 
> This could be extended pretty easily to offer more faked behavior, but i 
> stopped at providing an offer loop.  With this, jobs can be submitted, and 
> show as moving to RUNNING.
> 
> As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
> use the same approach.
> 
> 
> Diffs
> -
> 
>   build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7178a924ef8d7966bf24fa96657ea514080b1d00 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25257/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew run, also ran directly in intellij
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Joshua Cohen 

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

Ship it!


Looks good to my untrained-scheduler-internals-eyes.


src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java


s/  / /



src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java


Any way to avoid magic numbers here (and would it be worthwhile)? As a n00b 
to the scheduler I've got no idea what these are ;).


- Joshua Cohen


On Sept. 2, 2014, 5:16 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25257/
> ---
> 
> (Updated Sept. 2, 2014, 5:16 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji.
> 
> 
> Bugs: AURORA-658
> https://issues.apache.org/jira/browse/AURORA-658
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The big improvement over the previous incantation of local scheduler mode is 
> that arguments like `testing_isolated_scheduler` don't leak into production 
> builds.  There is also less affordance made in SchedulerMain and modules for 
> testing mode - the behavior changes with modules rather than branches.
> 
> This could be extended pretty easily to offer more faked behavior, but i 
> stopped at providing an offer loop.  With this, jobs can be submitted, and 
> show as moving to RUNNING.
> 
> As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to 
> use the same approach.
> 
> 
> Diffs
> -
> 
>   build.gradle 66c8205c8354048c398623edf7ba75e721376b01 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7178a924ef8d7966bf24fa96657ea514080b1d00 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25257/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew run, also ran directly in intellij
> 
> 
> Thanks,
> 
> Bill Farner
> 
>