date:20150225

Review Request 31406: Adding possiblity to define the port where the health checker should run on

2015-02-25 Thread Florian Pfeiffer


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

Review request for Aurora.


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


Repository: aurora


Description
---

Adding the possiblity to define the port where the health checker should run on

Note: The added mesos_task to the task runner seems to make the old task object 
superfluous. I wasn't sure if I could remove the task object that easily 
without breaking stuff, though. 
If we will go the way, that we're separating graceful teardown from 
healthcheck, we probably need the mesos_task object as well


Diffs
-

  src/main/python/apache/aurora/config/schema/base.py 
a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
  src/main/python/apache/aurora/executor/common/health_checker.py 
60676ba0fbd8a218fe4309f07de28e2c66d54530 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 

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


Testing
---

test_end_to_end.sh


Thanks,

Florian Pfeiffer

Re: Review Request 31406: Adding possiblity to define the port where the health checker should run on

2015-02-25 Thread Aurora ReviewBot


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


Master (4d5885b) is red with this patch.
  ./build-support/jenkins/build.sh

 src.test.python.apache.aurora.client.cli.cron  
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.inspect   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.job   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.plugins   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.quota 
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.sla   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.supdate   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.task  
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.update
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.version   
 .   SUCCESS
 src.test.python.apache.aurora.client.config
 .   SUCCESS
 src.test.python.apache.aurora.client.factory   
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_aurora_job_key   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster_option   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_clusters 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_http_signaler
 .   SUCCESS
 src.test.python.apache.aurora.common.test_pex_version  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_shellify 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_transport
 .   SUCCESS
 src.test.python.apache.aurora.config.test_base 
 .   SUCCESS
 
src.test.python.apache.aurora.config.test_constraint_parsing
.   SUCCESS
 src.test.python.apache.aurora.config.test_loader   
 .   SUCCESS
 src.test.python.apache.aurora.config.test_thrift   
 .   SUCCESS
 
src.test.python.apache.aurora.executor.common.path_detector 
.   SUCCESS
 src.test.python.apache.aurora.executor.common.task_info
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_base   
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_vars   
 .   SUCCESS
 src.test.python.apache.aurora.executor.status_manager  
 .   SUCCESS
 src.test.python.apache.aurora.executor.thermos_task_runner 
 .   FAILURE
 src.test.python.apache.thermos.common.test_pathspec
 .   SUCCESS
 
src.test.python.apache.thermos.core.test_runner_integration 
.   SUCCESS
 src.test.python.apache.thermos.monitoring.test_disk
 .   SUCCESS
 
FAILURE

[31m
   FAILURE[0m


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

- Aurora ReviewBot


On Feb. 25, 2015, 10:57 a.m., Florian Pfeiffer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31406/
> ---
> 
> (Updated Feb. 25, 2015, 10:57 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-321
>

Review Request 31418: Add admin port support in Mname.

2015-02-25 Thread Tony Dong


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

Review request for Aurora.


Repository: aurora


Description
---

Add admin port support in Mname.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/Mname.java 
69ce2aa3bcb494997fb655a1b51585aebc28f235 
  src/test/java/org/apache/aurora/scheduler/http/MnameTest.java 
afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5 

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


Testing
---


Thanks,

Tony Dong

Re: Review Request 31418: Add admin port support in mname.

2015-02-25 Thread Tony Dong


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

(Updated Feb. 25, 2015, 5:48 p.m.)


Review request for Aurora and Bill Farner.


Summary (updated)
-

Add admin port support in mname.


Repository: aurora


Description
---

Add admin port support in Mname.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/Mname.java 
69ce2aa3bcb494997fb655a1b51585aebc28f235 
  src/test/java/org/apache/aurora/scheduler/http/MnameTest.java 
afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5 

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


Testing
---


Thanks,

Tony Dong

Re: Review Request 31418: Add admin port support in Mname.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (4d5885b) 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 Feb. 25, 2015, 5:36 p.m., Tony Dong wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31418/
> ---
> 
> (Updated Feb. 25, 2015, 5:36 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add admin port support in Mname.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> 69ce2aa3bcb494997fb655a1b51585aebc28f235 
>   src/test/java/org/apache/aurora/scheduler/http/MnameTest.java 
> afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5 
> 
> Diff: https://reviews.apache.org/r/31418/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Dong
> 
>

Re: Review Request 31418: Add admin port support in mname.

2015-02-25 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Feb. 25, 2015, 5:48 p.m., Tony Dong wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31418/
> ---
> 
> (Updated Feb. 25, 2015, 5:48 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add admin port support in Mname.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> 69ce2aa3bcb494997fb655a1b51585aebc28f235 
>   src/test/java/org/apache/aurora/scheduler/http/MnameTest.java 
> afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5 
> 
> Diff: https://reviews.apache.org/r/31418/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Dong
> 
>

Review Request 31421: Handle TASK_ERROR and TASK_STAGING states.

2015-02-25 Thread Bill Farner


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

Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


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


Repository: aurora


Description
---

This is only a stop-gap to make sure we are compatible with mesos 0.22.  See 
ticket for more detail.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/Conversions.java 
7b7650d74e32d6396ab75a413380666d5000eac3 
  src/test/java/org/apache/aurora/scheduler/base/ConversionsTest.java 
PRE-CREATION 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31421: Handle TASK_ERROR and TASK_STAGING states.

2015-02-25 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Feb. 25, 2015, 6:48 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31421/
> ---
> 
> (Updated Feb. 25, 2015, 6:48 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1001
> https://issues.apache.org/jira/browse/AURORA-1001
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is only a stop-gap to make sure we are compatible with mesos 0.22.  See 
> ticket for more detail.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Conversions.java 
> 7b7650d74e32d6396ab75a413380666d5000eac3 
>   src/test/java/org/apache/aurora/scheduler/base/ConversionsTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31421/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31421: Handle TASK_ERROR and TASK_STAGING states.

2015-02-25 Thread Kevin Sweeney


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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 25, 2015, 10:48 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31421/
> ---
> 
> (Updated Feb. 25, 2015, 10:48 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1001
> https://issues.apache.org/jira/browse/AURORA-1001
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is only a stop-gap to make sure we are compatible with mesos 0.22.  See 
> ticket for more detail.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Conversions.java 
> 7b7650d74e32d6396ab75a413380666d5000eac3 
>   src/test/java/org/apache/aurora/scheduler/base/ConversionsTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31421/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31389: Remove LiveClusterState, make CachedClusterState (now ClusterStateImpl) default.

2015-02-25 Thread Joshua Cohen


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

Ship it!


Ship It!

- Joshua Cohen


On Feb. 25, 2015, 12:01 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31389/
> ---
> 
> (Updated Feb. 25, 2015, 12:01 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1055
> https://issues.apache.org/jira/browse/AURORA-1055
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove LiveClusterState, make CachedClusterState (now ClusterStateImpl) 
> default.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 0351b10cbfff76f07eec15c5ed1af7bb5f39a8ce 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterState.java
>  2831103457184d2049684c1c519b6bbb3b5b3e62 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterState.java
>  38cb17371735d959c14fc7bf9fc25b8eb814fdfe 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java
>  2030a9413035e1ad7db52b13957ed074b211d558 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterStateTest.java
>  7cc04dd4ea214f7e9923be51b467a2564fec18bb 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterStateTest.java
>  763f8b5f31349f291c0ede7b5d3292f6ca5b 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  83cf118c9e1d32b0de511754b4aeadd8aed33e49 
> 
> Diff: https://reviews.apache.org/r/31389/diff/
> 
> 
> Testing
> ---
> 
> Standard tests.  I removed a test case `testIgnoresThrottledTasks`, which was 
> actually testing LiveClusterState behavior.  That test case was redundant to 
> unit testing in `ClusterStateImpl`.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz


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

(Updated Feb. 25, 2015, 7:15 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


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


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

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


Testing
---


Thanks,

Steve Niemitz

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (81641a1) 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 Feb. 25, 2015, 7:15 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 7:15 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31421: Handle TASK_ERROR and TASK_STAGING states.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (81641a1) 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 Feb. 25, 2015, 6:48 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31421/
> ---
> 
> (Updated Feb. 25, 2015, 6:48 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1001
> https://issues.apache.org/jira/browse/AURORA-1001
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is only a stop-gap to make sure we are compatible with mesos 0.22.  See 
> ticket for more detail.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Conversions.java 
> 7b7650d74e32d6396ab75a413380666d5000eac3 
>   src/test/java/org/apache/aurora/scheduler/base/ConversionsTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31421/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Steve Niemitz


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

Review request for Aurora.


Repository: aurora


Description
---

Stop the announcer and status checkers before starting to kill the runners


Diffs
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
9c0282392dbb9cca308baf47adc1750c1f5cacc6 

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


Testing
---


Thanks,

Steve Niemitz

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Aurora ReviewBot


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


Master (895d03b) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Feb. 25, 2015, 8:17 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 25, 2015, 8:17 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner


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

(Updated Feb. 25, 2015, 8:37 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Updated console and javascript clients to use the new fields.


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


Repository: aurora


Description
---

Update thrift API and internal code to use JobUpdateSummary.key rather than job 
key and id.

Note: This change will leave the client in a broken state w.r.t. the scheduler. 
 I will hold commiting this change until i have another review ready to go that 
updates the client.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6a00ce2c30ce24851560c4d499c395194dbeed16 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
8444d225e91c75e9a571049d28af7264b6d2d017 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
6ade817284b4c0607fe1ff09ad61ea47768f7297 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
06de5384182f84f973a28e312c0dc4329f593dad 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
b9dce16fa705fc14b5af394f8edb6301c789aff4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
56398f33206cfa1fa45601a21760c7e23e2616bf 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
d1bd3c975b2057b46597f38555c2a73d14835600 
  src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
b8c919585307e27f154782ee179153165458bed7 
  src/main/python/apache/aurora/client/api/__init__.py 
07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
  src/main/python/apache/aurora/client/cli/context.py 
ea64010ab20c1292fb3fd1f5eb71394b7c965743 
  src/main/python/apache/aurora/client/cli/update.py 
bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 c08f09a511e1fc631abdae50630b8b7ab10a440b 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
  src/main/resources/scheduler/assets/breadcrumb.html 
b5f82ca120d3b76b63a61628437525cec8870015 
  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b79b07c23e00fab524aef21481070c5f09c9fa18 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
e969b972806bef9aa67729c8a35283df6145e687 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
2405bb8ad379e355a7987e9bb4163e1693f18777 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
3ba6274b97393ac2228d3eac8ba1615cb57014f8 
  src/test/python/apache/aurora/client/api/test_api.py 
ff1aff2eac391f219bc7c2483a16e35f916a224c 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
6413c0db20db5914c7b1537da2bbc6e110705dc2 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner


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

(Updated Feb. 25, 2015, 8:58 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Fixed checkstyle errors in python code.


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


Repository: aurora


Description
---

Update thrift API and internal code to use JobUpdateSummary.key rather than job 
key and id.

Note: This change will leave the client in a broken state w.r.t. the scheduler. 
 I will hold commiting this change until i have another review ready to go that 
updates the client.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6a00ce2c30ce24851560c4d499c395194dbeed16 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
8444d225e91c75e9a571049d28af7264b6d2d017 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
6ade817284b4c0607fe1ff09ad61ea47768f7297 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
06de5384182f84f973a28e312c0dc4329f593dad 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
b9dce16fa705fc14b5af394f8edb6301c789aff4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
56398f33206cfa1fa45601a21760c7e23e2616bf 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
d1bd3c975b2057b46597f38555c2a73d14835600 
  src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
b8c919585307e27f154782ee179153165458bed7 
  src/main/python/apache/aurora/client/api/__init__.py 
07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
  src/main/python/apache/aurora/client/cli/context.py 
ea64010ab20c1292fb3fd1f5eb71394b7c965743 
  src/main/python/apache/aurora/client/cli/update.py 
bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 c08f09a511e1fc631abdae50630b8b7ab10a440b 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
  src/main/resources/scheduler/assets/breadcrumb.html 
b5f82ca120d3b76b63a61628437525cec8870015 
  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b79b07c23e00fab524aef21481070c5f09c9fa18 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
e969b972806bef9aa67729c8a35283df6145e687 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
2405bb8ad379e355a7987e9bb4163e1693f18777 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
3ba6274b97393ac2228d3eac8ba1615cb57014f8 
  src/test/python/apache/aurora/client/api/test_api.py 
ff1aff2eac391f219bc7c2483a16e35f916a224c 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
6413c0db20db5914c7b1537da2bbc6e110705dc2 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Aurora ReviewBot


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


Master (895d03b) is red with this patch.
  ./build-support/jenkins/build.sh

SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_admin_util.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_maintenance.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_admin.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_admin_sla.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/util.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_transport.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_cluster_option.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_cluster.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_clusters.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_pex_version.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_shellify.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_aurora_job_key.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_http_signaler.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_constraint_parsing.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_thrift.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_loader.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_base.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_status_manager.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_executor_vars.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_executor_base.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_thermos_executor.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_gc_executor.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_thermos_task_runner.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_path_detector.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_announcer.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_kill_manager.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/__init__.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_executor_timeout.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/fixtures.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_health_checker.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_status_checker.py
 Everything Looks Good!
SUCCESS: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_resource_manager.py
 Everything Looks Good!
SUCCESS: 
/home

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Steve Niemitz


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

(Updated Feb. 25, 2015, 9:16 p.m.)


Review request for Aurora, Brian Wickman and Zameer Manji.


Repository: aurora


Description
---

Stop the announcer and status checkers before starting to kill the runners


Diffs
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
9c0282392dbb9cca308baf47adc1750c1f5cacc6 

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


Testing (updated)
---

We're now running this in our production environments.  Watching ZK, I can 
confirm that the nodes are removed before process shutdown begins.  Watching 
the executor log also confirms this.

I couldn't observe any other side effects either.


Thanks,

Steve Niemitz

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Steve Niemitz


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

(Updated Feb. 25, 2015, 9:22 p.m.)


Review request for Aurora, Brian Wickman and Zameer Manji.


Repository: aurora


Description (updated)
---

Stop the announcer and status checkers before starting to kill the runners.

This allows the task to be removed from the ZK ensemble before it begins 
getting killed.  The delay can be significant if the task takes some time to 
shutdown, and during the time it stops responding to requests.


Diffs
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
9c0282392dbb9cca308baf47adc1750c1f5cacc6 

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


Testing
---

We're now running this in our production environments.  Watching ZK, I can 
confirm that the nodes are removed before process shutdown begins.  Watching 
the executor log also confirms this.

I couldn't observe any other side effects either.


Thanks,

Steve Niemitz

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner


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


@ReviewBot retry

The last reply was not for the latest diff.

- Bill Farner


On Feb. 25, 2015, 8:58 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 8:58 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Zameer Manji


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

Ship it!


I think it is logical to shut down the status checkers before shutting down the 
task.

- Zameer Manji


On Feb. 25, 2015, 1:22 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 25, 2015, 1:22 p.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins 
> getting killed.  The delay can be significant if the task takes some time to 
> shutdown, and during the time it stops responding to requests.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> We're now running this in our production environments.  Watching ZK, I can 
> confirm that the nodes are removed before process shutdown begins.  Watching 
> the executor log also confirms this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Joshua Cohen


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


lgtm minus a few nits...


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


this should be unnecessary, we will have thrown on the previous line?

Also can you add a test for too many parts as well (e.g. foo:bar:baz:quux)?



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


Again, should be unnecessary.



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java


move to next line.

Also is this potentially fragile to assume the global mount will be the 
second mount in the list? It'd be slightly more robust to iterate the mounts 
and ensure the mount we expect is *somewhere* in the list?


- Joshua Cohen


On Feb. 25, 2015, 7:15 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 7:15 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Joshua Cohen


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


Is it worth adding test coverage for this shutdown ordering?

- Joshua Cohen


On Feb. 25, 2015, 9:22 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 25, 2015, 9:22 p.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins 
> getting killed.  The delay can be significant if the task takes some time to 
> shutdown, and during the time it stops responding to requests.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> We're now running this in our production environments.  Watching ZK, I can 
> confirm that the nodes are removed before process shutdown begins.  Watching 
> the executor log also confirms this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz



> On Feb. 25, 2015, 10:06 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java, lines 
> > 44-45
> > 
> >
> > this should be unnecessary, we will have thrown on the previous line?
> > 
> > Also can you add a test for too many parts as well (e.g. 
> > foo:bar:baz:quux)?

I added these in to stop some complaints about unused variables, but in 
retrospect I could have just called vp.doParse(...) and not assigned it.  I'll 
change these.


> On Feb. 25, 2015, 10:06 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
> >  line 214
> > 
> >
> > move to next line.
> > 
> > Also is this potentially fragile to assume the global mount will be the 
> > second mount in the list? It'd be slightly more robust to iterate the 
> > mounts and ensure the mount we expect is *somewhere* in the list?

Good point on the ordering, it shouldn't matter.  I'll change it.


- Steve


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


On Feb. 25, 2015, 7:15 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 7:15 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Aurora ReviewBot


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


Master (9442e08) is red with this patch.
  ./build-support/jenkins/build.sh

Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.collections-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.util
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.util-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.log
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.log-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.process
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.process-0.3.0-py2.7-nspkg.pth
  Running setup.py install for gitdb
building 'gitdb._perf' extension
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c 
gitdb/_fun.c -o build/temp.linux-x86_64-2.7/gitdb/_fun.o
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c 
gitdb/_delta_apply.c -o build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions 
-Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv 
-O2 -Wall -Wstrict-prototypes -D_FORTIFY_SOURCE=2 -g -fstack-protector 
--param=ssp-buffer-size=4 -Wformat -Werror=format-security 
build/temp.linux-x86_64-2.7/gitdb/_fun.o 
build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o -o 
build/lib.linux-x86_64-2.7/gitdb/_perf.so
  Running setup.py install for twitter.common.app
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.app-0.3.0-py2.7-nspkg.pth
  Running setup.py install for GitPython

/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/local/lib/python2.7/site-packages/setuptools/dist.py:292:
 UserWarning: The version specified ('0.3.2 RC1') is an invalid version, this 
may not work as expected with newer versions of setuptools, pip, and PyPI. 
Please see PEP 440 for more details.
  "details." % self.metadata.version
  Running setup.py install for pep8
Installing pep8 script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
  Running setup.py install for pyflakes
Installing pyflakes script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
  Running setup.py install for twitter.checkstyle
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.checkstyle-0.1.0-py2.7-nspkg.pth
Installing twitterstyle script to 
/home/jenki

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Zameer Manji



> On Feb. 25, 2015, 2:15 p.m., Joshua Cohen wrote:
> > Is it worth adding test coverage for this shutdown ordering?

I don't think mock allows us to test for order. Steve can you take a quick look 
and tell us if it is possible?


- Zameer


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


On Feb. 25, 2015, 1:22 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 25, 2015, 1:22 p.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins 
> getting killed.  The delay can be significant if the task takes some time to 
> shutdown, and during the time it stops responding to requests.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> We're now running this in our production environments.  Watching ZK, I can 
> confirm that the nodes are removed before process shutdown begins.  Watching 
> the executor log also confirms this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz


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

(Updated Feb. 25, 2015, 10:30 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


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


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

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


Testing
---


Thanks,

Steve Niemitz

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Aurora ReviewBot


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


Master (9442e08) is red with this patch.
  ./build-support/jenkins/build.sh

:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2

:processResources
:classes
:jar
:assemble
:compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation 
processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source 
'1.7'
1 warning

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain
:compileTestJava
:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java:22:15:
 Unused import - org.junit.Assert.assertNull.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java:45:
 Wrong order for 'java.util.List' import. Order should be: java, javax, scala, 
com, net, org. Each group should be separated by a single blank line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java:45:8:
 Unused import - java.util.List.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleTest'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/test.xml

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 1 mins 45.319 secs


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

- Aurora ReviewBot


On Feb. 25, 2015, 10:30 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 10:30 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Joshua Cohen


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

Ship it!


Looks good to me! I'll let someone a little bit more familiar with the 
container stuff chime in to make sure I'm not missing anything in that regard 
though.

- Joshua Cohen


On Feb. 25, 2015, 10:30 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 10:30 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz


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

(Updated Feb. 25, 2015, 10:35 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


Changes
---

That's what I get for not running tests on trivial changes...


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


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

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


Testing
---


Thanks,

Steve Niemitz

Review Request 31445: Expose more details about the tasks the preemptor is working for.

2015-02-25 Thread Bill Farner


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

Review request for Aurora, Kevin Sweeney and Zameer Manji.


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


Repository: aurora


Description
---

Expose more details about the tasks the preemptor is working for.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
 44cd8f79493f0f247cd876ef78b30b4f813314c4 
  src/test/java/org/apache/aurora/scheduler/testing/FakeStatsProvider.java 
768e78424268513742ecea22b2f5395dcb46da8c 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31445: Expose more details about the tasks the preemptor is working for.

2015-02-25 Thread Zameer Manji


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

Ship it!



src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java


I think some JavaDoc explaining which stats are exported would be nice. 
This would enable an operator/user to grep the source to see which code is 
responsible for a stat.


- Zameer Manji


On Feb. 25, 2015, 2:45 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31445/
> ---
> 
> (Updated Feb. 25, 2015, 2:45 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-524
> https://issues.apache.org/jira/browse/AURORA-524
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Expose more details about the tasks the preemptor is working for.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  44cd8f79493f0f247cd876ef78b30b4f813314c4 
>   src/test/java/org/apache/aurora/scheduler/testing/FakeStatsProvider.java 
> 768e78424268513742ecea22b2f5395dcb46da8c 
> 
> Diff: https://reviews.apache.org/r/31445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31445: Expose more details about the tasks the preemptor is working for.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (9442e08) 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 Feb. 25, 2015, 10:45 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31445/
> ---
> 
> (Updated Feb. 25, 2015, 10:45 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-524
> https://issues.apache.org/jira/browse/AURORA-524
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Expose more details about the tasks the preemptor is working for.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  44cd8f79493f0f247cd876ef78b30b4f813314c4 
>   src/test/java/org/apache/aurora/scheduler/testing/FakeStatsProvider.java 
> 768e78424268513742ecea22b2f5395dcb46da8c 
> 
> Diff: https://reviews.apache.org/r/31445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner


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

(Updated Feb. 25, 2015, 10:59 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Fixed checkstyle error.


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


Repository: aurora


Description
---

Update thrift API and internal code to use JobUpdateSummary.key rather than job 
key and id.

Note: This change will leave the client in a broken state w.r.t. the scheduler. 
 I will hold commiting this change until i have another review ready to go that 
updates the client.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6a00ce2c30ce24851560c4d499c395194dbeed16 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
8444d225e91c75e9a571049d28af7264b6d2d017 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
6ade817284b4c0607fe1ff09ad61ea47768f7297 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
06de5384182f84f973a28e312c0dc4329f593dad 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
b9dce16fa705fc14b5af394f8edb6301c789aff4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
56398f33206cfa1fa45601a21760c7e23e2616bf 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
d1bd3c975b2057b46597f38555c2a73d14835600 
  src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
b8c919585307e27f154782ee179153165458bed7 
  src/main/python/apache/aurora/client/api/__init__.py 
07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
  src/main/python/apache/aurora/client/cli/context.py 
ea64010ab20c1292fb3fd1f5eb71394b7c965743 
  src/main/python/apache/aurora/client/cli/update.py 
bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 c08f09a511e1fc631abdae50630b8b7ab10a440b 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
  src/main/resources/scheduler/assets/breadcrumb.html 
b5f82ca120d3b76b63a61628437525cec8870015 
  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b79b07c23e00fab524aef21481070c5f09c9fa18 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
e969b972806bef9aa67729c8a35283df6145e687 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
2405bb8ad379e355a7987e9bb4163e1693f18777 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
3ba6274b97393ac2228d3eac8ba1615cb57014f8 
  src/test/python/apache/aurora/client/api/test_api.py 
ff1aff2eac391f219bc7c2483a16e35f916a224c 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
6413c0db20db5914c7b1537da2bbc6e110705dc2 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Maxim Khutornenko


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


This does not seem to address js invocations of the job update APIs. E.g:

https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174

- Maxim Khutornenko


On Feb. 25, 2015, 10:59 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 10:59 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Aurora ReviewBot


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


Master (9442e08) is red with this patch.
  ./build-support/jenkins/build.sh

:checkstyleJmh
:jsHint
:checkstyleMain
:compileTestJava
:processTestResources
:testClasses
:checkstyleTest
:findbugsJmh
:findbugsMain
:findbugsTest
:licenseJmh UP-TO-DATE
:licenseMain UP-TO-DATE
:licenseTest UP-TO-DATE
:license UP-TO-DATE
:pmdMain
:test

org.apache.aurora.scheduler.http.ServletFilterTest > testGzipEncoding FAILED
java.lang.AssertionError at ServletFilterTest.java:49

799 tests completed, 1 failed, 1 skipped
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:analyzeReport
Instruction coverage of 0.8936328406633021 exceeds minimum coverage of 0.89.
Branch coverage of 0.8356164383561644 exceeds minimum coverage of 0.835.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/index.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 4 mins 15.948 secs


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

- Aurora ReviewBot


On Feb. 25, 2015, 10:35 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 10:35 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner


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

(Updated Feb. 25, 2015, 11:04 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


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


Repository: aurora


Description (updated)
---

Update thrift API and internal code to use JobUpdateSummary.key rather than job 
key and id.

Note: This change will leave the client in a broken state w.r.t. the scheduler. 
 I will hold commiting this change until i have another review ready to go that 
updates the client.

TODO: Update javascript uses of the API
https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174


Diffs
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6a00ce2c30ce24851560c4d499c395194dbeed16 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
8444d225e91c75e9a571049d28af7264b6d2d017 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
6ade817284b4c0607fe1ff09ad61ea47768f7297 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
06de5384182f84f973a28e312c0dc4329f593dad 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
b9dce16fa705fc14b5af394f8edb6301c789aff4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
56398f33206cfa1fa45601a21760c7e23e2616bf 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
d1bd3c975b2057b46597f38555c2a73d14835600 
  src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
b8c919585307e27f154782ee179153165458bed7 
  src/main/python/apache/aurora/client/api/__init__.py 
07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
  src/main/python/apache/aurora/client/cli/context.py 
ea64010ab20c1292fb3fd1f5eb71394b7c965743 
  src/main/python/apache/aurora/client/cli/update.py 
bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 c08f09a511e1fc631abdae50630b8b7ab10a440b 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
  src/main/resources/scheduler/assets/breadcrumb.html 
b5f82ca120d3b76b63a61628437525cec8870015 
  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b79b07c23e00fab524aef21481070c5f09c9fa18 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
e969b972806bef9aa67729c8a35283df6145e687 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
2405bb8ad379e355a7987e9bb4163e1693f18777 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
3ba6274b97393ac2228d3eac8ba1615cb57014f8 
  src/test/python/apache/aurora/client/api/test_api.py 
ff1aff2eac391f219bc7c2483a16e35f916a224c 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
6413c0db20db5914c7b1537da2bbc6e110705dc2 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner



> On Feb. 25, 2015, 11:01 p.m., Maxim Khutornenko wrote:
> > This does not seem to address js invocations of the job update APIs. E.g:
> > 
> > https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> > https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174

Thanks.  Mind doing a review in parallel with me addressing those?


- Bill


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


On Feb. 25, 2015, 10:59 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 10:59 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz


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


@ReviewBot retry

- Steve Niemitz


On Feb. 25, 2015, 10:35 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 10:35 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (9442e08) 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 Feb. 25, 2015, 11:04 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 11:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> TODO: Update javascript uses of the API
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Zameer Manji


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



src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java


Will this be addresed as apart of this effort?



src/main/python/apache/aurora/client/cli/update.py


I think the code is authored in this way to take advantage of the insert 
order of the dict. I think this was desired so the serialized string will have 
keys in certain order.



src/test/python/apache/aurora/client/cli/test_supdate.py


Instead of this todo, can you please replace the equality check with 
json.loads(mock_context.get_out_str()) == json.loads(""fixture"")

Even nicer would be to make the LHS a python dict to compare against the 
json.loads.


- Zameer Manji


On Feb. 25, 2015, 3:04 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 3:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> TODO: Update javascript uses of the API
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f9

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Maxim Khutornenko



> On Feb. 25, 2015, 11:01 p.m., Maxim Khutornenko wrote:
> > This does not seem to address js invocations of the job update APIs. E.g:
> > 
> > https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> > https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> Bill Farner wrote:
> Thanks.  Mind doing a review in parallel with me addressing those?

Rest lgtm.


- Maxim


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


On Feb. 25, 2015, 11:04 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 11:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> TODO: Update javascript uses of the API
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (9442e08) 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 Feb. 25, 2015, 10:35 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> ---
> 
> (Updated Feb. 25, 2015, 10:35 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
> https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31445: Expose more details about the tasks the preemptor is working for.

2015-02-25 Thread Bill Farner



> On Feb. 25, 2015, 10:54 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java,
> >  line 92
> > 
> >
> > I think some JavaDoc explaining which stats are exported would be nice. 
> > This would enable an operator/user to grep the source to see which code is 
> > responsible for a stat.

Overall -1 to that as there's nothing preventing drift.  I think it's generally 
safe to assume that operators understand that strings may be concatenated and 
grepping for specific parts may be required.


- Bill


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


On Feb. 25, 2015, 10:45 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31445/
> ---
> 
> (Updated Feb. 25, 2015, 10:45 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-524
> https://issues.apache.org/jira/browse/AURORA-524
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Expose more details about the tasks the preemptor is working for.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  44cd8f79493f0f247cd876ef78b30b4f813314c4 
>   src/test/java/org/apache/aurora/scheduler/testing/FakeStatsProvider.java 
> 768e78424268513742ecea22b2f5395dcb46da8c 
> 
> Diff: https://reviews.apache.org/r/31445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner



> On Feb. 25, 2015, 11:27 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java,
> >  line 219
> > 
> >
> > Will this be addresed as apart of this effort?

It actually is, this is a rogue TODO.  Thanks, removed.


> On Feb. 25, 2015, 11:27 p.m., Zameer Manji wrote:
> > src/main/python/apache/aurora/client/cli/update.py, line 269
> > 
> >
> > I think the code is authored in this way to take advantage of the 
> > insert order of the dict. I think this was desired so the serialized string 
> > will have keys in certain order.

Thanks.  Along with addressing your comment below, i have removed the TODO and 
comment.


> On Feb. 25, 2015, 11:27 p.m., Zameer Manji wrote:
> > src/test/python/apache/aurora/client/cli/test_supdate.py, line 359
> > 
> >
> > Instead of this todo, can you please replace the equality check with 
> > json.loads(mock_context.get_out_str()) == json.loads(""fixture"")
> > 
> > Even nicer would be to make the LHS a python dict to compare against 
> > the json.loads.

Thanks for the nudge, this was straightforward and did not result in a large 
code delta at all.  Fixed this and other similar tests.


- Bill


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


On Feb. 25, 2015, 11:04 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 11:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> TODO: Update javascript uses of the API
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Bill Farner


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

(Updated Feb. 26, 2015, 1:39 a.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


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


Repository: aurora


Description
---

Update thrift API and internal code to use JobUpdateSummary.key rather than job 
key and id.

Note: This change will leave the client in a broken state w.r.t. the scheduler. 
 I will hold commiting this change until i have another review ready to go that 
updates the client.

TODO: Update javascript uses of the API
https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6a00ce2c30ce24851560c4d499c395194dbeed16 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
8444d225e91c75e9a571049d28af7264b6d2d017 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
6ade817284b4c0607fe1ff09ad61ea47768f7297 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
06de5384182f84f973a28e312c0dc4329f593dad 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
b9dce16fa705fc14b5af394f8edb6301c789aff4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
56398f33206cfa1fa45601a21760c7e23e2616bf 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
d1bd3c975b2057b46597f38555c2a73d14835600 
  src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
b8c919585307e27f154782ee179153165458bed7 
  src/main/python/apache/aurora/client/api/__init__.py 
07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
  src/main/python/apache/aurora/client/cli/context.py 
ea64010ab20c1292fb3fd1f5eb71394b7c965743 
  src/main/python/apache/aurora/client/cli/update.py 
bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 c08f09a511e1fc631abdae50630b8b7ab10a440b 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
  src/main/resources/scheduler/assets/breadcrumb.html 
b5f82ca120d3b76b63a61628437525cec8870015 
  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
b79b07c23e00fab524aef21481070c5f09c9fa18 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
e969b972806bef9aa67729c8a35283df6145e687 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
2405bb8ad379e355a7987e9bb4163e1693f18777 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
3ba6274b97393ac2228d3eac8ba1615cb57014f8 
  src/test/python/apache/aurora/client/api/test_api.py 
ff1aff2eac391f219bc7c2483a16e35f916a224c 
  src/test/python/apache/aurora/client/cli/test_status.py 
1f39aa1c50a41e174804751b4cce25b96b6aebed 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
6413c0db20db5914c7b1537da2bbc6e110705dc2 

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


Testing
---


Thanks,

Bill Farner

Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Brian Wickman


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

Review request for Aurora, Joe Smith and Zameer Manji.


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


Repository: aurora


Description
---

This creates a new abstraction, the ObserverTaskDetector, which is responsible 
for managing state transitions for tasks for the observer.  Adds some tests and 
better debug logging.


Diffs
-

  src/main/python/apache/aurora/executor/common/resource_manager.py 
08e02e41b581f275f070228bb23c4cf2a0489f9a 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
  src/main/python/apache/thermos/bin/thermos.py 
0853a9892399824385bee9e72db4c108f46fceda 
  src/main/python/apache/thermos/common/path.py 
846f507e2e097fc04fe0098a7250b40fefcfc6e2 
  src/main/python/apache/thermos/monitoring/disk.py 
175ed3af6515e6107e297d91d4e30cbb3034faf7 
  src/main/python/apache/thermos/monitoring/monitor.py 
11423bc1764c8380d8de4ad095c1e2748ebb78f8 
  src/main/python/apache/thermos/monitoring/resource.py 
b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
  src/main/python/apache/thermos/observer/BUILD 
ee65f3a46e1d339620e76cadae92c6678fc3510f 
  src/main/python/apache/thermos/observer/bin/BUILD 
15a03f74f204f58856f0843b9db05e83b89d1138 
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
effa8c19f963bf2792497f4a06049214ae30dfa5 
  src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
  src/main/python/apache/thermos/observer/http/file_browser.py 
87ef9c8a29689c78a5e39a46cc53e4675e36a381 
  src/main/python/apache/thermos/observer/observed_task.py 
f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
  src/main/python/apache/thermos/observer/task_observer.py 
cd528dcca3f5a330359cf38005f3a1a0329a4886 
  src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
  src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 

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


Testing
---

Manually launched observer and ran some thermos tasks.
+
mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::


Thanks,

Brian Wickman

Review Request 31453: Run python style checks before unit tests in build.sh.

2015-02-25 Thread Bill Farner


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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

No major gain here, but when iterating locally, it's nice to not wait ~10 
minutes to get a style error.


Diffs
-

  build-support/jenkins/build.sh 61b5760f0b1e857bfd247b6488d6a7e9638aa679 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Brian Wickman


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



src/main/python/apache/thermos/observer/task_observer.py


fix this.


- Brian Wickman


On Feb. 26, 2015, 1:40 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> ---
> 
> (Updated Feb. 26, 2015, 1:40 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.
> 
> 
> Bugs: AURORA-1026
> https://issues.apache.org/jira/browse/AURORA-1026
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This creates a new abstraction, the ObserverTaskDetector, which is 
> responsible for managing state transitions for tasks for the observer.  Adds 
> some tests and better debug logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> 08e02e41b581f275f070228bb23c4cf2a0489f9a 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
>   src/main/python/apache/thermos/bin/thermos.py 
> 0853a9892399824385bee9e72db4c108f46fceda 
>   src/main/python/apache/thermos/common/path.py 
> 846f507e2e097fc04fe0098a7250b40fefcfc6e2 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 175ed3af6515e6107e297d91d4e30cbb3034faf7 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 11423bc1764c8380d8de4ad095c1e2748ebb78f8 
>   src/main/python/apache/thermos/monitoring/resource.py 
> b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
>   src/main/python/apache/thermos/observer/BUILD 
> ee65f3a46e1d339620e76cadae92c6678fc3510f 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 15a03f74f204f58856f0843b9db05e83b89d1138 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> effa8c19f963bf2792497f4a06049214ae30dfa5 
>   src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 87ef9c8a29689c78a5e39a46cc53e4675e36a381 
>   src/main/python/apache/thermos/observer/observed_task.py 
> f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
>   src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31451/diff/
> 
> 
> Testing
> ---
> 
> Manually launched observer and ran some thermos tasks.
> +
> mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Brian Wickman


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

(Updated Feb. 26, 2015, 1:50 a.m.)


Review request for Aurora, Joe Smith and Zameer Manji.


Changes
---

Fix missing reference to pathspec.


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


Repository: aurora


Description
---

This creates a new abstraction, the ObserverTaskDetector, which is responsible 
for managing state transitions for tasks for the observer.  Adds some tests and 
better debug logging.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/common/resource_manager.py 
08e02e41b581f275f070228bb23c4cf2a0489f9a 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
  src/main/python/apache/thermos/bin/thermos.py 
0853a9892399824385bee9e72db4c108f46fceda 
  src/main/python/apache/thermos/common/path.py 
846f507e2e097fc04fe0098a7250b40fefcfc6e2 
  src/main/python/apache/thermos/monitoring/disk.py 
175ed3af6515e6107e297d91d4e30cbb3034faf7 
  src/main/python/apache/thermos/monitoring/monitor.py 
11423bc1764c8380d8de4ad095c1e2748ebb78f8 
  src/main/python/apache/thermos/monitoring/resource.py 
b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
  src/main/python/apache/thermos/observer/BUILD 
ee65f3a46e1d339620e76cadae92c6678fc3510f 
  src/main/python/apache/thermos/observer/bin/BUILD 
15a03f74f204f58856f0843b9db05e83b89d1138 
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
effa8c19f963bf2792497f4a06049214ae30dfa5 
  src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
  src/main/python/apache/thermos/observer/http/file_browser.py 
87ef9c8a29689c78a5e39a46cc53e4675e36a381 
  src/main/python/apache/thermos/observer/observed_task.py 
f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
  src/main/python/apache/thermos/observer/task_observer.py 
cd528dcca3f5a330359cf38005f3a1a0329a4886 
  src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
  src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 

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


Testing
---

Manually launched observer and ran some thermos tasks.
+
mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::


Thanks,

Brian Wickman

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Brian Wickman



> On Feb. 26, 2015, 1:44 a.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 592
> > 
> >
> > fix this.

You should definitely be concerned about the current state of the observer.  
I've started a big refactor and some of the progress can be found here: 
https://github.com/apache/incubator-aurora/compare/master...wickman:wickman/AURORA-1026
  It will substantially improve the test coverage for this code.


- Brian


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


On Feb. 26, 2015, 1:50 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> ---
> 
> (Updated Feb. 26, 2015, 1:50 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.
> 
> 
> Bugs: AURORA-1026
> https://issues.apache.org/jira/browse/AURORA-1026
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This creates a new abstraction, the ObserverTaskDetector, which is 
> responsible for managing state transitions for tasks for the observer.  Adds 
> some tests and better debug logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> 08e02e41b581f275f070228bb23c4cf2a0489f9a 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
>   src/main/python/apache/thermos/bin/thermos.py 
> 0853a9892399824385bee9e72db4c108f46fceda 
>   src/main/python/apache/thermos/common/path.py 
> 846f507e2e097fc04fe0098a7250b40fefcfc6e2 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 175ed3af6515e6107e297d91d4e30cbb3034faf7 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 11423bc1764c8380d8de4ad095c1e2748ebb78f8 
>   src/main/python/apache/thermos/monitoring/resource.py 
> b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
>   src/main/python/apache/thermos/observer/BUILD 
> ee65f3a46e1d339620e76cadae92c6678fc3510f 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 15a03f74f204f58856f0843b9db05e83b89d1138 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> effa8c19f963bf2792497f4a06049214ae30dfa5 
>   src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 87ef9c8a29689c78a5e39a46cc53e4675e36a381 
>   src/main/python/apache/thermos/observer/observed_task.py 
> f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
>   src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31451/diff/
> 
> 
> Testing
> ---
> 
> Manually launched observer and ran some thermos tasks.
> +
> mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31453: Run python style checks before unit tests in build.sh.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (9442e08) 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 Feb. 26, 2015, 1:41 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31453/
> ---
> 
> (Updated Feb. 26, 2015, 1:41 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> No major gain here, but when iterating locally, it's nice to not wait ~10 
> minutes to get a style error.
> 
> 
> Diffs
> -
> 
>   build-support/jenkins/build.sh 61b5760f0b1e857bfd247b6488d6a7e9638aa679 
> 
> Diff: https://reviews.apache.org/r/31453/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 31456: Only fetch host attributes once per preemption round.

2015-02-25 Thread Bill Farner


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

Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


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


Repository: aurora


Description
---

A small, but cheap performance improvement when the preemptor is engaged.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
42af883721e4e5c0ae23ff65a5fb7dc285e48faa 

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


Testing
---

Before this diff:

```
o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
   avgt  100   5788175.295 ±  47410.511  ns/op
o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
 avgt  100   5046038.818 ±  38636.961  ns/op
o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
avgt  100  64323073.493 ± 644053.437  ns/op
```

After this diff:
```
o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
   avgt  100   5783859.480 ±  37291.662  ns/op
o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
 avgt  100   5259022.679 ±  30276.902  ns/op
o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
avgt  100  58171450.071 ± 638305.308  ns/op
```

So, about 10% for ~free.


Thanks,

Bill Farner

Re: Review Request 31456: Only fetch host attributes once per preemption round.

2015-02-25 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Feb. 26, 2015, 2 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31456/
> ---
> 
> (Updated Feb. 26, 2015, 2 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1153
> https://issues.apache.org/jira/browse/AURORA-1153
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A small, but cheap performance improvement when the preemptor is engaged.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
> 
> Diff: https://reviews.apache.org/r/31456/diff/
> 
> 
> Testing
> ---
> 
> Before this diff:
> 
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5788175.295 ±  47410.511  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5046038.818 ±  38636.961  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  64323073.493 ± 644053.437  ns/op
> ```
> 
> After this diff:
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5783859.480 ±  37291.662  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5259022.679 ±  30276.902  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  58171450.071 ± 638305.308  ns/op
> ```
> 
> So, about 10% for ~free.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Aurora ReviewBot


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


Master (9442e08) is red with this patch.
  ./build-support/jenkins/build.sh

 src.test.python.apache.aurora.client.cli.version   
 .   SUCCESS
 src.test.python.apache.aurora.client.config
 .   SUCCESS
 src.test.python.apache.aurora.client.factory   
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_aurora_job_key   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster_option   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_clusters 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_http_signaler
 .   SUCCESS
 src.test.python.apache.aurora.common.test_pex_version  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_shellify 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_transport
 .   SUCCESS
 src.test.python.apache.aurora.config.test_base 
 .   SUCCESS
 
src.test.python.apache.aurora.config.test_constraint_parsing
.   SUCCESS
 src.test.python.apache.aurora.config.test_loader   
 .   SUCCESS
 src.test.python.apache.aurora.config.test_thrift   
 .   SUCCESS
 src.test.python.apache.aurora.executor.common.announcer
 .   SUCCESS
 
src.test.python.apache.aurora.executor.common.directory_sandbox 
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.executor_detector 
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.executor_timeout  
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.health_checker
.   SUCCESS
 src.test.python.apache.aurora.executor.common.kill_manager 
 .   SUCCESS
 
src.test.python.apache.aurora.executor.common.path_detector 
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.status_checker
.   SUCCESS
 src.test.python.apache.aurora.executor.common.task_info
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_base   
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_vars   
 .   SUCCESS
 src.test.python.apache.aurora.executor.gc_executor 
 .   SUCCESS
 src.test.python.apache.aurora.executor.status_manager  
 .   SUCCESS
 src.test.python.apache.aurora.executor.thermos_executor
 .   FAILURE
 src.test.python.apache.aurora.executor.thermos_task_runner 
 .   SUCCESS
 src.test.python.apache.thermos.common.test_pathspec
 .   SUCCESS
 
src.test.python.apache.thermos.core.test_runner_integration 
.   SUCCESS
 src.test.python.apache.thermos.monitoring.test_disk
 .   SUCCESS
 
FAILURE

[31m
   FAILURE[0m


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

- Aurora ReviewBot


On Feb. 26, 2015, 1:50 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> ---
> 
> (Updated Feb. 26, 2015, 1:50 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Steve Niemitz


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

(Updated Feb. 26, 2015, 2:14 a.m.)


Review request for Aurora, Brian Wickman and Zameer Manji.


Changes
---

This fails the stylechecker because of the multiline with clause, but there's 
no better way to do it that I can see?

Anyways, this adds a unit test to enforce the call order to be 
status_checker.stop() -> runner.stop()


Repository: aurora


Description
---

Stop the announcer and status checkers before starting to kill the runners.

This allows the task to be removed from the ZK ensemble before it begins 
getting killed.  The delay can be significant if the task takes some time to 
shutdown, and during the time it stops responding to requests.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
9c0282392dbb9cca308baf47adc1750c1f5cacc6 
  src/test/python/apache/aurora/executor/BUILD 
2ee9b1233e9db47455ddffbc48691d379222 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
8dbfb1db5eb7a6548820ff7cf82a9c7092f61d28 

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


Testing
---

We're now running this in our production environments.  Watching ZK, I can 
confirm that the nodes are removed before process shutdown begins.  Watching 
the executor log also confirms this.

I couldn't observe any other side effects either.


Thanks,

Steve Niemitz

Re: Review Request 31456: Only fetch host attributes once per preemption round.

2015-02-25 Thread Kevin Sweeney


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

Ship it!


Ship It!

- Kevin Sweeney


On Feb. 25, 2015, 6 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31456/
> ---
> 
> (Updated Feb. 25, 2015, 6 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1153
> https://issues.apache.org/jira/browse/AURORA-1153
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A small, but cheap performance improvement when the preemptor is engaged.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
> 
> Diff: https://reviews.apache.org/r/31456/diff/
> 
> 
> Testing
> ---
> 
> Before this diff:
> 
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5788175.295 ±  47410.511  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5046038.818 ±  38636.961  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  64323073.493 ± 644053.437  ns/op
> ```
> 
> After this diff:
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5783859.480 ±  37291.662  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5259022.679 ±  30276.902  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  58171450.071 ± 638305.308  ns/op
> ```
> 
> So, about 10% for ~free.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Aurora ReviewBot


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

Ship it!


Master (9442e08) 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 Feb. 26, 2015, 1:39 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 26, 2015, 1:39 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> TODO: Update javascript uses of the API
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 1f39aa1c50a41e174804751b4cce25b96b6aebed 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Aurora ReviewBot


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


Master (9442e08) is red with this patch.
  ./build-support/jenkins/build.sh

Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.util-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.log
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.log-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.process
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.process-0.3.0-py2.7-nspkg.pth
  Running setup.py install for gitdb
building 'gitdb._perf' extension
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c 
gitdb/_fun.c -o build/temp.linux-x86_64-2.7/gitdb/_fun.o
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c 
gitdb/_delta_apply.c -o build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions 
-Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv 
-O2 -Wall -Wstrict-prototypes -D_FORTIFY_SOURCE=2 -g -fstack-protector 
--param=ssp-buffer-size=4 -Wformat -Werror=format-security 
build/temp.linux-x86_64-2.7/gitdb/_fun.o 
build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o -o 
build/lib.linux-x86_64-2.7/gitdb/_perf.so
  Running setup.py install for twitter.common.app
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.app-0.3.0-py2.7-nspkg.pth
  Running setup.py install for GitPython

/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/local/lib/python2.7/site-packages/setuptools/dist.py:292:
 UserWarning: The version specified ('0.3.2 RC1') is an invalid version, this 
may not work as expected with newer versions of setuptools, pip, and PyPI. 
Please see PEP 440 for more details.
  "details." % self.metadata.version
  Running setup.py install for pep8
Installing pep8 script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
  Running setup.py install for pyflakes
Installing pyflakes script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
  Running setup.py install for twitter.checkstyle
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.checkstyle-0.1.0-py2.7-nspkg.pth
Installing twitterstyle script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Successfully installed GitPython-0.3.2rc1 gitdb-0.6.4 pep8-1.4.5 pyflakes-0.7.2 
smmap-0.9.0 twitter.checkstyle-0.1.0 twitter.common.app-0.3.0 
twitter

Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.

2015-02-25 Thread Zameer Manji


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

Ship it!


Ship It!

- Zameer Manji


On Feb. 25, 2015, 5:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31388/
> ---
> 
> (Updated Feb. 25, 2015, 5:39 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update thrift API and internal code to use JobUpdateSummary.key rather than 
> job key and id.
> 
> Note: This change will leave the client in a broken state w.r.t. the 
> scheduler.  I will hold commiting this change until i have another review 
> ready to go that updates the client.
> 
> TODO: Update javascript uses of the API
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/controllers.js#L279
> https://github.com/apache/incubator-aurora/blob/master/src/main/resources/scheduler/assets/js/services.js#L174
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6a00ce2c30ce24851560c4d499c395194dbeed16 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8444d225e91c75e9a571049d28af7264b6d2d017 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 6ade817284b4c0607fe1ff09ad61ea47768f7297 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 06de5384182f84f973a28e312c0dc4329f593dad 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> b9dce16fa705fc14b5af394f8edb6301c789aff4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  56398f33206cfa1fa45601a21760c7e23e2616bf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  d1bd3c975b2057b46597f38555c2a73d14835600 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> b8c919585307e27f154782ee179153165458bed7 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 07d1b1c5405b1329a15e8900b07c2a3dba6c592f 
>   src/main/python/apache/aurora/client/cli/context.py 
> ea64010ab20c1292fb3fd1f5eb71394b7c965743 
>   src/main/python/apache/aurora/client/cli/update.py 
> bfa7a27c4e283b981a4ce37d382aa117a0074ee1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  c08f09a511e1fc631abdae50630b8b7ab10a440b 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> e6184dd2de0e86849d811ec05c168bbb4db8e8cd 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> b5f82ca120d3b76b63a61628437525cec8870015 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b79b07c23e00fab524aef21481070c5f09c9fa18 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  81c8be7685854c47ed4a7c6deeffb6a0b80023ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  e969b972806bef9aa67729c8a35283df6145e687 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d36418807ace4f61e8709a2827cea0e7bb8ac6b7 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2405bb8ad379e355a7987e9bb4163e1693f18777 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 3ba6274b97393ac2228d3eac8ba1615cb57014f8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> ff1aff2eac391f219bc7c2483a16e35f916a224c 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 1f39aa1c50a41e174804751b4cce25b96b6aebed 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 6413c0db20db5914c7b1537da2bbc6e110705dc2 
> 
> Diff: https://reviews.apache.org/r/31388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Zameer Manji



> On Feb. 25, 2015, 5:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 592
> > 
> >
> > fix this.
> 
> Brian Wickman wrote:
> You should definitely be concerned about the current state of the 
> observer.  I've started a big refactor and some of the progress can be found 
> here: 
> https://github.com/apache/incubator-aurora/compare/master...wickman:wickman/AURORA-1026
>   It will substantially improve the test coverage for this code.

Would it be too much to ask to land the refactor in pieces first before this 
change?


- Zameer


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


On Feb. 25, 2015, 5:50 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> ---
> 
> (Updated Feb. 25, 2015, 5:50 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.
> 
> 
> Bugs: AURORA-1026
> https://issues.apache.org/jira/browse/AURORA-1026
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This creates a new abstraction, the ObserverTaskDetector, which is 
> responsible for managing state transitions for tasks for the observer.  Adds 
> some tests and better debug logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> 08e02e41b581f275f070228bb23c4cf2a0489f9a 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
>   src/main/python/apache/thermos/bin/thermos.py 
> 0853a9892399824385bee9e72db4c108f46fceda 
>   src/main/python/apache/thermos/common/path.py 
> 846f507e2e097fc04fe0098a7250b40fefcfc6e2 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 175ed3af6515e6107e297d91d4e30cbb3034faf7 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 11423bc1764c8380d8de4ad095c1e2748ebb78f8 
>   src/main/python/apache/thermos/monitoring/resource.py 
> b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
>   src/main/python/apache/thermos/observer/BUILD 
> ee65f3a46e1d339620e76cadae92c6678fc3510f 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 15a03f74f204f58856f0843b9db05e83b89d1138 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> effa8c19f963bf2792497f4a06049214ae30dfa5 
>   src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 87ef9c8a29689c78a5e39a46cc53e4675e36a381 
>   src/main/python/apache/thermos/observer/observed_task.py 
> f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
>   src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31451/diff/
> 
> 
> Testing
> ---
> 
> Manually launched observer and ran some thermos tasks.
> +
> mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31453: Run python style checks before unit tests in build.sh.

2015-02-25 Thread Zameer Manji


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

Ship it!


Ship It!

- Zameer Manji


On Feb. 25, 2015, 5:41 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31453/
> ---
> 
> (Updated Feb. 25, 2015, 5:41 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> No major gain here, but when iterating locally, it's nice to not wait ~10 
> minutes to get a style error.
> 
> 
> Diffs
> -
> 
>   build-support/jenkins/build.sh 61b5760f0b1e857bfd247b6488d6a7e9638aa679 
> 
> Diff: https://reviews.apache.org/r/31453/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31456: Only fetch host attributes once per preemption round.

2015-02-25 Thread Aurora ReviewBot


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


Master (9442e08) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Feb. 26, 2015, 2 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31456/
> ---
> 
> (Updated Feb. 26, 2015, 2 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1153
> https://issues.apache.org/jira/browse/AURORA-1153
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A small, but cheap performance improvement when the preemptor is engaged.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
> 42af883721e4e5c0ae23ff65a5fb7dc285e48faa 
> 
> Diff: https://reviews.apache.org/r/31456/diff/
> 
> 
> Testing
> ---
> 
> Before this diff:
> 
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5788175.295 ±  47410.511  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5046038.818 ±  38636.961  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  64323073.493 ± 644053.437  ns/op
> ```
> 
> After this diff:
> ```
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
>avgt  100   5783859.480 ±  37291.662  ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>  avgt  100   5259022.679 ±  30276.902  ns/op
> o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
> avgt  100  58171450.071 ± 638305.308  ns/op
> ```
> 
> So, about 10% for ~free.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Brian Wickman



> On Feb. 26, 2015, 1:44 a.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 592
> > 
> >
> > fix this.
> 
> Brian Wickman wrote:
> You should definitely be concerned about the current state of the 
> observer.  I've started a big refactor and some of the progress can be found 
> here: 
> https://github.com/apache/incubator-aurora/compare/master...wickman:wickman/AURORA-1026
>   It will substantially improve the test coverage for this code.
> 
> Zameer Manji wrote:
> Would it be too much to ask to land the refactor in pieces first before 
> this change?

yes.


- Brian


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


On Feb. 26, 2015, 1:50 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> ---
> 
> (Updated Feb. 26, 2015, 1:50 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.
> 
> 
> Bugs: AURORA-1026
> https://issues.apache.org/jira/browse/AURORA-1026
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This creates a new abstraction, the ObserverTaskDetector, which is 
> responsible for managing state transitions for tasks for the observer.  Adds 
> some tests and better debug logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> 08e02e41b581f275f070228bb23c4cf2a0489f9a 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
>   src/main/python/apache/thermos/bin/thermos.py 
> 0853a9892399824385bee9e72db4c108f46fceda 
>   src/main/python/apache/thermos/common/path.py 
> 846f507e2e097fc04fe0098a7250b40fefcfc6e2 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 175ed3af6515e6107e297d91d4e30cbb3034faf7 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 11423bc1764c8380d8de4ad095c1e2748ebb78f8 
>   src/main/python/apache/thermos/monitoring/resource.py 
> b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
>   src/main/python/apache/thermos/observer/BUILD 
> ee65f3a46e1d339620e76cadae92c6678fc3510f 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 15a03f74f204f58856f0843b9db05e83b89d1138 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> effa8c19f963bf2792497f4a06049214ae30dfa5 
>   src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 87ef9c8a29689c78a5e39a46cc53e4675e36a381 
>   src/main/python/apache/thermos/observer/observed_task.py 
> f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
>   src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31451/diff/
> 
> 
> Testing
> ---
> 
> Manually launched observer and ran some thermos tasks.
> +
> mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Joshua Cohen


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



src/test/python/apache/aurora/executor/test_thermos_executor.py


You should be able to use contextlib.nested for this:

with contextlib.nested(
temporary_dir(),
mock.patch.object(...),
mock.patch.object(...)) as (
checkpoint_root, 
status_check_stop, 
runner_stop):

  # use all the things
  
Alternately, I think you can just wrap the with in parens as well, since 
we're pinned to python2.7 now (though a quick scan of the code doesn't reveal 
any place that we do that, but does show lots of places where we use 
contextlib.nested).



src/test/python/apache/aurora/executor/test_thermos_executor.py


Is it worth being explicit and passing any_order=False (even though it's 
the default), just to make it clear that the order is important?

I had to look it up to see what the behavior was, maybe for people more 
familiar with python/mock would know right away, but it's not abundantly clear.


- Joshua Cohen


On Feb. 26, 2015, 2:14 a.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 26, 2015, 2:14 a.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins 
> getting killed.  The delay can be significant if the task takes some time to 
> shutdown, and during the time it stops responding to requests.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
>   src/test/python/apache/aurora/executor/BUILD 
> 2ee9b1233e9db47455ddffbc48691d379222 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 8dbfb1db5eb7a6548820ff7cf82a9c7092f61d28 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> We're now running this in our production environments.  Watching ZK, I can 
> confirm that the nodes are removed before process shutdown begins.  Watching 
> the executor log also confirms this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Steve Niemitz



> On Feb. 26, 2015, 3:38 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, lines 
> > 384-386
> > 
> >
> > You should be able to use contextlib.nested for this:
> > 
> > with contextlib.nested(
> > temporary_dir(),
> > mock.patch.object(...),
> > mock.patch.object(...)) as (
> > checkpoint_root, 
> > status_check_stop, 
> > runner_stop):
> > 
> >   # use all the things
> >   
> > Alternately, I think you can just wrap the with in parens as well, 
> > since we're pinned to python2.7 now (though a quick scan of the code 
> > doesn't reveal any place that we do that, but does show lots of places 
> > where we use contextlib.nested).

I tried parens and they don't work with "with" for whatever reason (syntax 
error).  I tried to avoid contextlib.nested because it's deprecated as of 2.7, 
but I can use that if you guys think its ok.


> On Feb. 26, 2015, 3:38 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, line 398
> > 
> >
> > Is it worth being explicit and passing any_order=False (even though 
> > it's the default), just to make it clear that the order is important?
> > 
> > I had to look it up to see what the behavior was, maybe for people more 
> > familiar with python/mock would know right away, but it's not abundantly 
> > clear.

:thumbsup:


- Steve


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


On Feb. 26, 2015, 2:14 a.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 26, 2015, 2:14 a.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins 
> getting killed.  The delay can be significant if the task takes some time to 
> shutdown, and during the time it stops responding to requests.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
>   src/test/python/apache/aurora/executor/BUILD 
> 2ee9b1233e9db47455ddffbc48691d379222 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 8dbfb1db5eb7a6548820ff7cf82a9c7092f61d28 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> We're now running this in our production environments.  Watching ZK, I can 
> confirm that the nodes are removed before process shutdown begins.  Watching 
> the executor log also confirms this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31423: Stop the announcer and status checkers before starting to kill the runners

2015-02-25 Thread Joshua Cohen



> On Feb. 26, 2015, 3:38 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, lines 
> > 384-386
> > 
> >
> > You should be able to use contextlib.nested for this:
> > 
> > with contextlib.nested(
> > temporary_dir(),
> > mock.patch.object(...),
> > mock.patch.object(...)) as (
> > checkpoint_root, 
> > status_check_stop, 
> > runner_stop):
> > 
> >   # use all the things
> >   
> > Alternately, I think you can just wrap the with in parens as well, 
> > since we're pinned to python2.7 now (though a quick scan of the code 
> > doesn't reveal any place that we do that, but does show lots of places 
> > where we use contextlib.nested).
> 
> Steve Niemitz wrote:
> I tried parens and they don't work with "with" for whatever reason 
> (syntax error).  I tried to avoid contextlib.nested because it's deprecated 
> as of 2.7, but I can use that if you guys think its ok.

Huh, how about that, seems like a genuine wart that they don't seem interested 
in fixing: http://bugs.python.org/issue12782

Anyway, the tests are littered with contextlib.nested, so I say go for it!


- Joshua


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


On Feb. 26, 2015, 2:14 a.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31423/
> ---
> 
> (Updated Feb. 26, 2015, 2:14 a.m.)
> 
> 
> Review request for Aurora, Brian Wickman and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Stop the announcer and status checkers before starting to kill the runners.
> 
> This allows the task to be removed from the ZK ensemble before it begins 
> getting killed.  The delay can be significant if the task takes some time to 
> shutdown, and during the time it stops responding to requests.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 9c0282392dbb9cca308baf47adc1750c1f5cacc6 
>   src/test/python/apache/aurora/executor/BUILD 
> 2ee9b1233e9db47455ddffbc48691d379222 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 8dbfb1db5eb7a6548820ff7cf82a9c7092f61d28 
> 
> Diff: https://reviews.apache.org/r/31423/diff/
> 
> 
> Testing
> ---
> 
> We're now running this in our production environments.  Watching ZK, I can 
> confirm that the nodes are removed before process shutdown begins.  Watching 
> the executor log also confirms this.
> 
> I couldn't observe any other side effects either.
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 31451: Port thermos observer to the path detector interface

2015-02-25 Thread Zameer Manji


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



src/main/python/apache/thermos/bin/thermos.py


How is this change related to the observer change?



src/main/python/apache/thermos/observer/detector.py


Can you please confirm if this property is only used in testing?



src/main/python/apache/thermos/observer/detector.py


Please put a TODO(wickman) instead of a TBD



src/main/python/apache/thermos/observer/task_observer.py


Why is the detector protected witha lock but none of the callbacks are?

I think either the detector should have an internal lock to prevent 
concurrent refreshes or the lock here should also be used to protect the 
callbacks that are passed in.


- Zameer Manji


On Feb. 25, 2015, 5:50 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> ---
> 
> (Updated Feb. 25, 2015, 5:50 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.
> 
> 
> Bugs: AURORA-1026
> https://issues.apache.org/jira/browse/AURORA-1026
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This creates a new abstraction, the ObserverTaskDetector, which is 
> responsible for managing state transitions for tasks for the observer.  Adds 
> some tests and better debug logging.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> 08e02e41b581f275f070228bb23c4cf2a0489f9a 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
>   src/main/python/apache/thermos/bin/thermos.py 
> 0853a9892399824385bee9e72db4c108f46fceda 
>   src/main/python/apache/thermos/common/path.py 
> 846f507e2e097fc04fe0098a7250b40fefcfc6e2 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 175ed3af6515e6107e297d91d4e30cbb3034faf7 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 11423bc1764c8380d8de4ad095c1e2748ebb78f8 
>   src/main/python/apache/thermos/monitoring/resource.py 
> b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
>   src/main/python/apache/thermos/observer/BUILD 
> ee65f3a46e1d339620e76cadae92c6678fc3510f 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 15a03f74f204f58856f0843b9db05e83b89d1138 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> effa8c19f963bf2792497f4a06049214ae30dfa5 
>   src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 87ef9c8a29689c78a5e39a46cc53e4675e36a381 
>   src/main/python/apache/thermos/observer/observed_task.py 
> f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
>   src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31451/diff/
> 
> 
> Testing
> ---
> 
> Manually launched observer and ran some thermos tasks.
> +
> mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

69 matches

Mail list logo