Re: Review Request 29731: Service status endpoint.

2015-01-08 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Jan. 8, 2015, 11:47 p.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29731/
 ---
 
 (Updated Jan. 8, 2015, 11:47 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Service status endpoint for debugging.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/GuavaUtils.java 
 f3fa8cd9f9200e32ab5a81d2a174184faa49d0a3 
   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
 e741913c5c91bc3aba0a790759420f13a9ce00bd 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 8a7926b8c82b318e2eef3d6493b0e817d67c2a6f 
   src/main/java/org/apache/aurora/scheduler/http/Services.java PRE-CREATION 
   src/main/resources/scheduler/assets/index.html 
 442f10b359b121660cbc0c74205441e1d738645f 
   src/test/java/org/apache/aurora/scheduler/http/ServicesTest.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/29731/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 Inspected output with ./gradlew run
 
 ```
 % curl http://localhost:8081/services | python -m json.tool
 [
 {
 name: TaskTimeout,
 state: RUNNING
 },
 {
 name: JobUpdateHistoryPruner,
 state: RUNNING
 },
 {
 name: TaskStatUpdaterService,
 state: RUNNING
 },
 {
 name: SlotSizeCounterService,
 state: RUNNING
 },
 {
 name: SlaUpdater,
 state: RUNNING
 },
 {
 name: CronLifecycle,
 state: RUNNING
 },
 {
 name: TaskVars,
 state: RUNNING
 },
 {
 name: RegisterGauges,
 state: RUNNING
 },
 {
 name: RegisterSubscribers,
 state: RUNNING
 },
 {
 name: RedirectMonitor,
 state: RUNNING
 }
 ]
 ```
 
 
 Thanks,
 
 Kevin Sweeney
 




Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen

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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

Fix reviewbot to skip reviews that have no diffs.


Diffs
-

  build-support/jenkins/review_feedback.py 
bd2c9941960645f662ec835c2baa4d1f3dae7d79 

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


Testing
---

No... is there any way for me to test this?


Thanks,

Joshua Cohen



Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Kevin Sweeney

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


For testing you can run it locally.

- Kevin Sweeney


On Jan. 8, 2015, 4:04 p.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29734/
 ---
 
 (Updated Jan. 8, 2015, 4:04 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix reviewbot to skip reviews that have no diffs.
 
 We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) 
 of the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that 
 has no diffs. This should fix those cases.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 bd2c9941960645f662ec835c2baa4d1f3dae7d79 
 
 Diff: https://reviews.apache.org/r/29734/diff/
 
 
 Testing
 ---
 
 No... is there any way for me to test this?
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen

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

(Updated Jan. 9, 2015, 1:02 a.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

-wfarner, +ksweeney so we can ship this and quiet the noise from failed 
reviewbot builds.


Repository: aurora


Description
---

Fix reviewbot to skip reviews that have no diffs.

We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) of 
the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that has 
no diffs. This should fix those cases.


Diffs
-

  build-support/jenkins/review_feedback.py 
bd2c9941960645f662ec835c2baa4d1f3dae7d79 

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


Testing
---

Ran locally, it didn't crash on r28943


Thanks,

Joshua Cohen



Review Request 29731: Service status endpoint.

2015-01-08 Thread Kevin Sweeney

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

Service status endpoint for debugging.


Diffs
-

  src/main/java/org/apache/aurora/GuavaUtils.java 
f3fa8cd9f9200e32ab5a81d2a174184faa49d0a3 
  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
e741913c5c91bc3aba0a790759420f13a9ce00bd 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a7926b8c82b318e2eef3d6493b0e817d67c2a6f 
  src/main/java/org/apache/aurora/scheduler/http/Services.java PRE-CREATION 
  src/main/resources/scheduler/assets/index.html 
442f10b359b121660cbc0c74205441e1d738645f 
  src/test/java/org/apache/aurora/scheduler/http/ServicesTest.java PRE-CREATION 

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


Testing
---

./gradlew -Pq build

Inspected output with ./gradlew run

% curl http://localhost:8081/services | python -m json.tool
[
{
name: TaskTimeout,
state: RUNNING
},
{
name: JobUpdateHistoryPruner,
state: RUNNING
},
{
name: TaskStatUpdaterService,
state: RUNNING
},
{
name: SlotSizeCounterService,
state: RUNNING
},
{
name: SlaUpdater,
state: RUNNING
},
{
name: CronLifecycle,
state: RUNNING
},
{
name: TaskVars,
state: RUNNING
},
{
name: RegisterGauges,
state: RUNNING
},
{
name: RegisterSubscribers,
state: RUNNING
},
{
name: RedirectMonitor,
state: RUNNING
}
]


Thanks,

Kevin Sweeney



Re: Review Request 29731: Service status endpoint.

2015-01-08 Thread Kevin Sweeney

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

(Updated Jan. 8, 2015, 3:47 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

Service status endpoint for debugging.


Diffs
-

  src/main/java/org/apache/aurora/GuavaUtils.java 
f3fa8cd9f9200e32ab5a81d2a174184faa49d0a3 
  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
e741913c5c91bc3aba0a790759420f13a9ce00bd 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
8a7926b8c82b318e2eef3d6493b0e817d67c2a6f 
  src/main/java/org/apache/aurora/scheduler/http/Services.java PRE-CREATION 
  src/main/resources/scheduler/assets/index.html 
442f10b359b121660cbc0c74205441e1d738645f 
  src/test/java/org/apache/aurora/scheduler/http/ServicesTest.java PRE-CREATION 

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


Testing (updated)
---

./gradlew -Pq build

Inspected output with ./gradlew run

```
% curl http://localhost:8081/services | python -m json.tool
[
{
name: TaskTimeout,
state: RUNNING
},
{
name: JobUpdateHistoryPruner,
state: RUNNING
},
{
name: TaskStatUpdaterService,
state: RUNNING
},
{
name: SlotSizeCounterService,
state: RUNNING
},
{
name: SlaUpdater,
state: RUNNING
},
{
name: CronLifecycle,
state: RUNNING
},
{
name: TaskVars,
state: RUNNING
},
{
name: RegisterGauges,
state: RUNNING
},
{
name: RegisterSubscribers,
state: RUNNING
},
{
name: RedirectMonitor,
state: RUNNING
}
]
```


Thanks,

Kevin Sweeney



Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen

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

(Updated Jan. 9, 2015, 12:31 a.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

Fix reviewbot to skip reviews that have no diffs.

We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) of 
the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that has 
no diffs. This should fix those cases.


Diffs
-

  build-support/jenkins/review_feedback.py 
bd2c9941960645f662ec835c2baa4d1f3dae7d79 

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


Testing (updated)
---

Ran locally, it didn't crash on r28943


Thanks,

Joshua Cohen



Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen

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

(Updated Jan. 9, 2015, 12:38 a.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Fix my bad python.


Repository: aurora


Description
---

Fix reviewbot to skip reviews that have no diffs.

We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) of 
the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that has 
no diffs. This should fix those cases.


Diffs (updated)
-

  build-support/jenkins/review_feedback.py 
bd2c9941960645f662ec835c2baa4d1f3dae7d79 
  docs/client-commands.md 75e69541fd95dfd9a7aa1e04de1a590b8fcbeacf 
  docs/client.md 3ec39b4f3bd6b45692aa1291e66a0a171d7dbb68 
  src/main/python/apache/aurora/client/cli/__init__.py 
395819fdf24b7919b32be51060fb5b581c8e1514 
  src/main/python/apache/aurora/client/cli/client.py 
939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
  src/main/python/apache/aurora/client/cli/options.py 
b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
  src/main/python/apache/aurora/client/cli/task.py 
e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
  src/test/python/apache/aurora/client/cli/BUILD 
bbac5c8efc9892fd2a966a6ac25fe05ffd740733 
  src/test/python/apache/aurora/client/cli/test_help.py 
9fa05e683f01a0e51253e08aa7fba69fd49d3756 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
  src/test/python/apache/aurora/client/cli/util.py 
1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 

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


Testing
---

Ran locally, it didn't crash on r28943


Thanks,

Joshua Cohen



Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (4431297) 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 Jan. 9, 2015, 12:41 a.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29734/
 ---
 
 (Updated Jan. 9, 2015, 12:41 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix reviewbot to skip reviews that have no diffs.
 
 We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) 
 of the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that 
 has no diffs. This should fix those cases.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 bd2c9941960645f662ec835c2baa4d1f3dae7d79 
 
 Diff: https://reviews.apache.org/r/29734/diff/
 
 
 Testing
 ---
 
 Ran locally, it didn't crash on r28943
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen

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

(Updated Jan. 9, 2015, 12:04 a.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description (updated)
---

Fix reviewbot to skip reviews that have no diffs.

We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) of 
the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that has 
no diffs. This should fix those cases.


Diffs
-

  build-support/jenkins/review_feedback.py 
bd2c9941960645f662ec835c2baa4d1f3dae7d79 

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


Testing
---

No... is there any way for me to test this?


Thanks,

Joshua Cohen



Re: Review Request 29731: Service status endpoint.

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (4431297) 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 Jan. 8, 2015, 11:47 p.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29731/
 ---
 
 (Updated Jan. 8, 2015, 11:47 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Service status endpoint for debugging.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/GuavaUtils.java 
 f3fa8cd9f9200e32ab5a81d2a174184faa49d0a3 
   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
 e741913c5c91bc3aba0a790759420f13a9ce00bd 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 8a7926b8c82b318e2eef3d6493b0e817d67c2a6f 
   src/main/java/org/apache/aurora/scheduler/http/Services.java PRE-CREATION 
   src/main/resources/scheduler/assets/index.html 
 442f10b359b121660cbc0c74205441e1d738645f 
   src/test/java/org/apache/aurora/scheduler/http/ServicesTest.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/29731/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 Inspected output with ./gradlew run
 
 ```
 % curl http://localhost:8081/services | python -m json.tool
 [
 {
 name: TaskTimeout,
 state: RUNNING
 },
 {
 name: JobUpdateHistoryPruner,
 state: RUNNING
 },
 {
 name: TaskStatUpdaterService,
 state: RUNNING
 },
 {
 name: SlotSizeCounterService,
 state: RUNNING
 },
 {
 name: SlaUpdater,
 state: RUNNING
 },
 {
 name: CronLifecycle,
 state: RUNNING
 },
 {
 name: TaskVars,
 state: RUNNING
 },
 {
 name: RegisterGauges,
 state: RUNNING
 },
 {
 name: RegisterSubscribers,
 state: RUNNING
 },
 {
 name: RedirectMonitor,
 state: RUNNING
 }
 ]
 ```
 
 
 Thanks,
 
 Kevin Sweeney
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen


 On Jan. 9, 2015, 12:04 a.m., Kevin Sweeney wrote:
  For testing you can run it locally.

Thanks, ran locally and all looked well.


- Joshua


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


On Jan. 9, 2015, 12:04 a.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29734/
 ---
 
 (Updated Jan. 9, 2015, 12:04 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix reviewbot to skip reviews that have no diffs.
 
 We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) 
 of the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that 
 has no diffs. This should fix those cases.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 bd2c9941960645f662ec835c2baa4d1f3dae7d79 
 
 Diff: https://reviews.apache.org/r/29734/diff/
 
 
 Testing
 ---
 
 No... is there any way for me to test this?
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (66128e4) 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 Jan. 9, 2015, 12:04 a.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29734/
 ---
 
 (Updated Jan. 9, 2015, 12:04 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix reviewbot to skip reviews that have no diffs.
 
 We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) 
 of the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that 
 has no diffs. This should fix those cases.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 bd2c9941960645f662ec835c2baa4d1f3dae7d79 
 
 Diff: https://reviews.apache.org/r/29734/diff/
 
 
 Testing
 ---
 
 No... is there any way for me to test this?
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Joshua Cohen

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

(Updated Jan. 9, 2015, 12:41 a.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Fix diff.


Repository: aurora


Description
---

Fix reviewbot to skip reviews that have no diffs.

We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) of 
the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that has 
no diffs. This should fix those cases.


Diffs (updated)
-

  build-support/jenkins/review_feedback.py 
bd2c9941960645f662ec835c2baa4d1f3dae7d79 

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


Testing
---

Ran locally, it didn't crash on r28943


Thanks,

Joshua Cohen



Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Kevin Sweeney


 On Jan. 8, 2015, 10:48 a.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 155
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line155
 
  From reading the code, it appears this additional path is needed for 
  them both to be available in the container.  If so, would it be easier to 
  just accept an arbitrary number of additional assets to copy into the 
  sandbox?  I would find that more generalized, and easier to understand.
  
  If you go with the above, i _think_ you can also safely nuke the extra 
  args plumbing.
 
 Steve Niemitz wrote:
 I think we'll still need extra args (its really useful right now to pass 
 ZK config in), but I like the idea of an arbitrary set of resources.  The 
 only trouble here would be figuring out which is the one to run and getting 
 the symlinks right.  Let's talk about this more.
 
 Bill Farner wrote:
 Wouldn't the extra args just be solved with the shim script?  I tell the 
 scheduler to copy `my_executor.sh` and `executor.pex` into the container, and 
 `my_executor.sh` contains the extra args.  I like this as a generalized 
 solution to augmenting default executor behavior, since it avoids feature 
 creep on our side just to save people the shim script.
 
 Steve Niemitz wrote:
 Correct, it would.  My goal with this (which I'm happy to discuss more) 
 was to make the wrapper script unnecessary in normal cases.  For us we just 
 need to pass in the ZK config for the announcer, and I feel like this is the 
 typical case.  Let me simmer on this one for a little bit.

I'd like to entirely avoid the need for a shim script by configuring the 
scheduler. The rationale being I would like to one day be able to host executor 
builds on a public artifact store (that a user doesn't necessarily have write 
access to) with all aurora-specific configuration happening on the scheduler.

In that world the slaves would only need to know about the master, and 
site-specific executor settings could be configured on the scheduler like so:
```
-thermos_executor_path=https://some.public.url/thermos-0.7.0.pex
-thermos_executor_flags='--announcer-enable --announcer-ensemble=zk://... ...'
```


- Kevin


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


On Jan. 8, 2015, 12:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Jan. 8, 2015, 12:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample aurora file is in 
 examples/jobs/docker.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 More information is in subsequent review request comments.
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 08ba1cdf88b712de22c26c04443079282db59ef9 
   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/vagrant/aurorabuild.sh b7ea41719a8f41bb23d0254e732926d89399c77c 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
   src/main/python/apache/aurora/executor/common/sandbox.py 
 f47a32b3fefb4a89940b1ddc473b8316ac00df12 
   

Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (4431297) 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 Jan. 8, 2015, 9:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 9:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Jan. 8, 2015, 4:31 p.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29734/
 ---
 
 (Updated Jan. 8, 2015, 4:31 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix reviewbot to skip reviews that have no diffs.
 
 We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) 
 of the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that 
 has no diffs. This should fix those cases.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 bd2c9941960645f662ec835c2baa4d1f3dae7d79 
 
 Diff: https://reviews.apache.org/r/29734/diff/
 
 
 Testing
 ---
 
 Ran locally, it didn't crash on r28943
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 29734: Fix reviewbot to skip reviews that have no diffs.

2015-01-08 Thread Maxim Khutornenko

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

Ship it!



build-support/jenkins/review_feedback.py
https://reviews.apache.org/r/29734/#comment111341

if not diffs: should do the same


- Maxim Khutornenko


On Jan. 9, 2015, 12:31 a.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29734/
 ---
 
 (Updated Jan. 9, 2015, 12:31 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix reviewbot to skip reviews that have no diffs.
 
 We're seeing [failures](https://builds.apache.org/job/AuroraBot/724/console) 
 of the AuroraBot job for a [review](https://reviews.apache.org/r/28943/) that 
 has no diffs. This should fix those cases.
 
 
 Diffs
 -
 
   build-support/jenkins/review_feedback.py 
 bd2c9941960645f662ec835c2baa4d1f3dae7d79 
 
 Diff: https://reviews.apache.org/r/29734/diff/
 
 
 Testing
 ---
 
 Ran locally, it didn't crash on r28943
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 29464: Add option to override local scheduler address published into ZooKeeper

2015-01-08 Thread Steve Niemitz


 On Jan. 8, 2015, 2:20 a.m., Kevin Sweeney wrote:
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java, line 96
  https://reviews.apache.org/r/29464/diff/8/?file=809079#file809079line96
 
  requireNonNull

I didn't add this (I just reordered the parameters), so I'm hesitant to change 
it.


 On Jan. 8, 2015, 2:20 a.m., Kevin Sweeney wrote:
  src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java,
   line 83
  https://reviews.apache.org/r/29464/diff/8/?file=809080#file809080line83
 
  im not sure why this needs to be synchronized? holding an intrinsic 
  lock while calling out to another class is prone to deadlocks and I don't 
  see any mutable state being protected.

Good catch, I think this was an artifact of some previous refactoring.


 On Jan. 8, 2015, 2:20 a.m., Kevin Sweeney wrote:
  src/main/python/apache/aurora/executor/thermos_task_runner.py, line 76
  https://reviews.apache.org/r/29464/diff/8/?file=809084#file809084line76
 
  Can we drop the default here or make it lazy? Otherwise importing this 
  module has the side-effect of making a syscall.

That's how I had this origionally but I changed it based on [feedback 
above](#review66428).  You're going to get a syscall regardless though.


 On Jan. 8, 2015, 2:20 a.m., Kevin Sweeney wrote:
  src/main/python/apache/aurora/executor/thermos_task_runner.py, line 101
  https://reviews.apache.org/r/29464/diff/8/?file=809084#file809084line101
 
  convention here would seem to be to make hostname default to None and 
  do hostname or socket.gethostname() here.

Same as above.  I'll change it back...


- Steve


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


On Jan. 7, 2015, 9:21 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29464/
 ---
 
 (Updated Jan. 7, 2015, 9:21 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I've added a new flag for the aurora scheduler, -hostname which can override 
 the scheduler server address published into ZK.
 
 This is useful for cases such as running the scheduler in EC2, where the 
 autodetected local address is actual an interal IP and not the public address 
 of the machine.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 
 2c449a491bc5a8ac858ea6487e4cef0591f36f66 
   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
 360e161b6c3f6fd412c7e8de7f1b9a3af109593c 
   
 src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
  PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
 cf173850635572c0df38bdd5cb14de8ce2016bf7 
   src/main/python/apache/aurora/executor/common/announcer.py 
 9e5bdc3885e76d8d03aa946caac9fdec7e1e9186 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/bin/thermos_runner.py 
 647de2771f301b17de33d8b45198c211d2e84367 
   src/main/python/apache/thermos/core/runner.py 
 8aac6b50c66080abbb5308b367e9f74c487f42e3 
   
 src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
  PRE-CREATION 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 5e54364a49a208bd5f19b9649633dc8feca591e9 
   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
 fbc3da3ab239b67ce3012d5a14fccd3ccb20a241 
   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
 c3bf5ea4cbeaad03e187f84215b86531d55c25b3 
   src/test/python/apache/aurora/executor/common/test_announcer.py 
 e329a90b8fba43611f5120e2a5ee82220dbe2a91 
 
 Diff: https://reviews.apache.org/r/29464/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Maxim Khutornenko

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



src/test/python/apache/aurora/client/cli/test_inspect.py
https://reviews.apache.org/r/29696/#comment111091

Is it a copy-paste from somewhere? I don't see anything using waiting 
events around inspect command.


- Maxim Khutornenko


On Jan. 8, 2015, 2:19 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 2:19 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29464: Add option to override local scheduler address published into ZooKeeper

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (9a817f2) 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 Jan. 8, 2015, 5:25 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29464/
 ---
 
 (Updated Jan. 8, 2015, 5:25 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 I've added a new flag for the aurora scheduler, -hostname which can override 
 the scheduler server address published into ZK.
 
 This is useful for cases such as running the scheduler in EC2, where the 
 autodetected local address is actual an interal IP and not the public address 
 of the machine.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 
 2c449a491bc5a8ac858ea6487e4cef0591f36f66 
   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
 360e161b6c3f6fd412c7e8de7f1b9a3af109593c 
   
 src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
  PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
 cf173850635572c0df38bdd5cb14de8ce2016bf7 
   src/main/python/apache/aurora/executor/common/announcer.py 
 9e5bdc3885e76d8d03aa946caac9fdec7e1e9186 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/bin/thermos_runner.py 
 647de2771f301b17de33d8b45198c211d2e84367 
   src/main/python/apache/thermos/core/runner.py 
 8aac6b50c66080abbb5308b367e9f74c487f42e3 
   
 src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
  PRE-CREATION 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 5e54364a49a208bd5f19b9649633dc8feca591e9 
   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
 fbc3da3ab239b67ce3012d5a14fccd3ccb20a241 
   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
 c3bf5ea4cbeaad03e187f84215b86531d55c25b3 
   src/test/python/apache/aurora/executor/common/test_announcer.py 
 e329a90b8fba43611f5120e2a5ee82220dbe2a91 
 
 Diff: https://reviews.apache.org/r/29464/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 7, 2015, 7:33 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 7, 2015, 7:33 p.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
 Fully specified job instance key, in
 CLUSTER/ROLE/ENV/NAME[/INSTANCES] format. If INSTANCES
 is omitted, then all instances will be operated on.
   unix_command_line
 
 optional arguments:
   -h, --helpshow this help message and exit
   --threads NUM_THREADS, -t NUM_THREADS

Re: Review Request 29700: Remove CommandHooks policy mechanism and dynamic hooks.

2015-01-08 Thread Maxim Khutornenko

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


I am getting a timeout when getting to the diff. What's going on there that 
changes almost every file in a repo?

- Maxim Khutornenko


On Jan. 8, 2015, 4:01 a.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29700/
 ---
 
 (Updated Jan. 8, 2015, 4:01 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This patch removes the CommandHooks policy mechanism and dynamic hook 
 discovery. 
 
 
 Diffs
 -
 
   .auroraversion 0034eec93d9d40c8039735f01192121bd2edebea 
   .gitattributes 8d50d053e8aa5f4e54fc85c687f9869875d22e09 
   .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
   3rdparty/javascript/bower_components/angular-bootstrap/.bower.json  
   3rdparty/javascript/bower_components/angular-bootstrap/bower.json  
   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 
  
   
 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js
   
   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js  
   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js  
   3rdparty/javascript/bower_components/angular-route/.bower.json  
   3rdparty/javascript/bower_components/angular-route/README.md  
   3rdparty/javascript/bower_components/angular-route/angular-route.js  
   3rdparty/javascript/bower_components/angular-route/angular-route.min.js  
   3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 
  
   3rdparty/javascript/bower_components/angular-route/bower.json  
   3rdparty/javascript/bower_components/angular/.bower.json  
   3rdparty/javascript/bower_components/angular/README.md  
   3rdparty/javascript/bower_components/angular/angular-csp.css  
   3rdparty/javascript/bower_components/angular/angular.js  
   3rdparty/javascript/bower_components/angular/angular.min.js  
   3rdparty/javascript/bower_components/angular/angular.min.js.gzip  
   3rdparty/javascript/bower_components/angular/angular.min.js.map  
   3rdparty/javascript/bower_components/angular/bower.json  
   3rdparty/javascript/bower_components/bootstrap/.bower.json  
   3rdparty/javascript/bower_components/bootstrap/Gruntfile.js  
   3rdparty/javascript/bower_components/bootstrap/LICENSE  
   3rdparty/javascript/bower_components/bootstrap/README.md  
   3rdparty/javascript/bower_components/bootstrap/bower.json  
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 
  
   
 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css
   
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css  
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map  
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css  
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff
   
   3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js  
   3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js  
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.eot
   
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.svg
   
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.ttf
   
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.woff
   
   
 3rdparty/javascript/bower_components/bootstrap/grunt/bs-glyphicons-data-generator.js
   
   3rdparty/javascript/bower_components/bootstrap/grunt/bs-lessdoc-parser.js  
   
 3rdparty/javascript/bower_components/bootstrap/grunt/bs-raw-files-generator.js
   
   3rdparty/javascript/bower_components/bootstrap/grunt/shrinkwrap.js  
   3rdparty/javascript/bower_components/bootstrap/js/affix.js  
   3rdparty/javascript/bower_components/bootstrap/js/alert.js  
   3rdparty/javascript/bower_components/bootstrap/js/button.js  
   3rdparty/javascript/bower_components/bootstrap/js/carousel.js  
   3rdparty/javascript/bower_components/bootstrap/js/collapse.js  
   3rdparty/javascript/bower_components/bootstrap/js/dropdown.js  
   

Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Zameer Manji

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



src/test/python/apache/aurora/client/cli/test_inspect.py
https://reviews.apache.org/r/29696/#comment05

This test doesn't communicate with the scheduler so why do we mock this out?



src/test/python/apache/aurora/client/cli/test_inspect.py
https://reviews.apache.org/r/29696/#comment07

Is this patch needed?


- Zameer Manji


On Jan. 7, 2015, 6:19 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 7, 2015, 6:19 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29700: Remove CommandHooks policy mechanism and dynamic hooks.

2015-01-08 Thread Zameer Manji


 On Jan. 8, 2015, 10:02 a.m., Maxim Khutornenko wrote:
  I am getting a timeout when getting to the diff. What's going on there that 
  changes almost every file in a repo?

It seems that reviewboard was having issues and some how mangled my patch. I 
will abandon this diff and resubmit.


- Zameer


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


On Jan. 7, 2015, 8:01 p.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29700/
 ---
 
 (Updated Jan. 7, 2015, 8:01 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This patch removes the CommandHooks policy mechanism and dynamic hook 
 discovery. 
 
 
 Diffs
 -
 
   .auroraversion 0034eec93d9d40c8039735f01192121bd2edebea 
   .gitattributes 8d50d053e8aa5f4e54fc85c687f9869875d22e09 
   .gitignore ad4206d179b46f1269fd58c62de9e6e62f4b8738 
   3rdparty/javascript/bower_components/angular-bootstrap/.bower.json  
   3rdparty/javascript/bower_components/angular-bootstrap/bower.json  
   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js 
  
   
 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js
   
   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js  
   3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js  
   3rdparty/javascript/bower_components/angular-route/.bower.json  
   3rdparty/javascript/bower_components/angular-route/README.md  
   3rdparty/javascript/bower_components/angular-route/angular-route.js  
   3rdparty/javascript/bower_components/angular-route/angular-route.min.js  
   3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map 
  
   3rdparty/javascript/bower_components/angular-route/bower.json  
   3rdparty/javascript/bower_components/angular/.bower.json  
   3rdparty/javascript/bower_components/angular/README.md  
   3rdparty/javascript/bower_components/angular/angular-csp.css  
   3rdparty/javascript/bower_components/angular/angular.js  
   3rdparty/javascript/bower_components/angular/angular.min.js  
   3rdparty/javascript/bower_components/angular/angular.min.js.gzip  
   3rdparty/javascript/bower_components/angular/angular.min.js.map  
   3rdparty/javascript/bower_components/angular/bower.json  
   3rdparty/javascript/bower_components/bootstrap/.bower.json  
   3rdparty/javascript/bower_components/bootstrap/Gruntfile.js  
   3rdparty/javascript/bower_components/bootstrap/LICENSE  
   3rdparty/javascript/bower_components/bootstrap/README.md  
   3rdparty/javascript/bower_components/bootstrap/bower.json  
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css 
  
   
 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css
   
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css  
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map  
   3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css  
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf
   
   
 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff
   
   3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js  
   3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js  
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.eot
   
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.svg
   
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.ttf
   
   
 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.woff
   
   
 3rdparty/javascript/bower_components/bootstrap/grunt/bs-glyphicons-data-generator.js
   
   3rdparty/javascript/bower_components/bootstrap/grunt/bs-lessdoc-parser.js  
   
 3rdparty/javascript/bower_components/bootstrap/grunt/bs-raw-files-generator.js
   
   3rdparty/javascript/bower_components/bootstrap/grunt/shrinkwrap.js  
   3rdparty/javascript/bower_components/bootstrap/js/affix.js  
   3rdparty/javascript/bower_components/bootstrap/js/alert.js  
   3rdparty/javascript/bower_components/bootstrap/js/button.js  
   3rdparty/javascript/bower_components/bootstrap/js/carousel.js  
   

Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Maxim Khutornenko

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

Ship it!



src/main/python/apache/aurora/client/cli/task.py
https://reviews.apache.org/r/29698/#comment15

The majority of subcommand help strings (i.e. jobs.py) start with capital 
letter. Suggest making it consistent here.


- Maxim Khutornenko


On Jan. 8, 2015, 3:33 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 3:33 a.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
 Fully specified job instance key, in
 CLUSTER/ROLE/ENV/NAME[/INSTANCES] format. If INSTANCES
   

Review Request 29717: Remove dynamic command hooks and dynamic hook policy.

2015-01-08 Thread Zameer Manji

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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

This patch removes the dynamic hooks functionality and dynamic hook policy 
mechanism from the client. As far as I can tell they are currently unused and 
add a lot of complexity to the client.


Diffs
-

  docs/design/command-hooks.md cc56218625edfd1b255f1ca18ae91c32e2dcccef 
  src/main/python/apache/aurora/client/cli/__init__.py 
6e553d8af459e575b2d62282a3bc0d1e266203d8 
  src/main/python/apache/aurora/client/cli/command_hooks.py 
aa850bf941bede1d3bd8aae4811cb094ba77965f 
  src/test/python/apache/aurora/client/cli/AuroraHooks 
e27fcc81d6092b3b42f9a2948e3955d8f6963a14 
  
src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax/AuroraHooks 
9a221591250f4d0d3b1b75f90b8b4cf8f95ee4b9 
  
src/test/python/apache/aurora/client/cli/hook_test_data/exec_error/AuroraHooks 
5dc5907b9ae87632f91084e43be319c6f1b4f437 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
3acd2ba0d8bd8c71d4c0a9d71a035fc974fa20c3 

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


Testing
---

./pants build --timeout=60 
src/test/python/apache/aurora/client/cli:command_hooks -vxs


Thanks,

Zameer Manji



Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Jan. 8, 2015, 3:33 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 3:33 a.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
 Fully specified job instance key, in
 CLUSTER/ROLE/ENV/NAME[/INSTANCES] format. If INSTANCES
 is omitted, then all instances will be operated on.
   unix_command_line
 
 optional arguments:
   -h, --helpshow this help message and exit
   --threads NUM_THREADS, -t NUM_THREADS

Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Bill Farner

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


Great patch overall!  Mostly documentation and style nits, but also opening 
some questions.


api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment36

For this doc and others, i found the docs in .md to be more informative, 
please lift them here.  e.g., this was more helpful: `The path inside the 
container where the mount will be created.`.



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment00

This should be camelCase, ditto below.



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment10

This may be my docker ignorance showing, but can you expand on this doc?  
It's not blatantly obvious what this is for and how it differs from 
`container_path`.



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment33

What are the implications of this?  Does it mean a docker image must be 
pre-loaded on the host for it to be used?  Can we cope with the user specifying 
a bad path?  If not, what's the fallout - TASK_FAILED?



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment16

Perhaps this should be called `imageName`?  Without help in the name, i 
could be convinced this was a path or URI.

That said, what is this for?  As a user, can i incorrectly specify/format 
this string, or is it for my own purposes?



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment27

s/A set of z/Z/

As a convention, when type information is already expressed in a 
signature/declaration, tend away from repeating type information in the doc.



docs/configuration-reference.md
https://reviews.apache.org/r/28920/#comment55

rephrase suggestion:

`*Note: the only container type currently supported is docker*`

(italicized, on its own line at the top)



docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/28920/#comment62

It would be really useful to see a working example of these configuration 
parameters in concert.  I suggest you go ahead and wire it up in the upstart 
configs we have, and link to them from this doc.

Taking it a step further, it would be awesome if this was exercised in 
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`.



docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/28920/#comment58

linkify docker containerizer to 
http://mesos.apache.org/documentation/latest/docker-containerizer/



docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/28920/#comment59

s/aurora/mesos/?  IIUC it's the slave that does the copy.



docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/28920/#comment84

From reading the code, it appears this additional path is needed for them 
both to be available in the container.  If so, would it be easier to just 
accept an arbitrary number of additional assets to copy into the sandbox?  I 
would find that more generalized, and easier to understand.

If you go with the above, i _think_ you can also safely nuke the extra args 
plumbing.



src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
https://reviews.apache.org/r/28920/#comment96

Style nit: we usually leave one blank line between a wrapped method 
signature and the body, for better visual separation.



src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
https://reviews.apache.org/r/28920/#comment92

I applied your patch and removed these PMD exclusions without any issue.  
Are they needed?

As a general practice - we avoid decorating the code with hints to code 
quality checkers.  This could vary from making the code appease the checker to 
disabling the rule.



src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
https://reviews.apache.org/r/28920/#comment82

It's good form for this type of sanitization to happen here, but at minimum 
the same sanitization must be done in `ConfigurationManager` to give the user a 
good message and avoid accepting the bad config.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment99

Declare these as Strings, do the Optional.of(X) where used.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment97

requireNonNull



src/main/python/apache/aurora/executor/bin/thermos_executor_main.py
https://reviews.apache.org/r/28920/#comment111201

typo: specified



src/main/resources/scheduler/assets/js/services.js

Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Kevin Sweeney

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

Ship it!



src/main/python/apache/aurora/client/cli/task.py
https://reviews.apache.org/r/29698/#comment111209

since these aren't multiline anymore you don't need the triple quotes. In 
fact, you can do

```py
help = ...
```

instead of @property.


- Kevin Sweeney


On Jan. 7, 2015, 7:33 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 7, 2015, 7:33 p.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
 Fully specified job instance key, in
 CLUSTER/ROLE/ENV/NAME[/INSTANCES] 

Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Zameer Manji

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



src/test/python/apache/aurora/client/cli/test_inspect.py
https://reviews.apache.org/r/29696/#comment111278

I'm confused, if we define the print function here aren't we just testing 
that our print function that we define here prints what we think it does?



src/test/python/apache/aurora/client/cli/test_inspect.py
https://reviews.apache.org/r/29696/#comment111277

Do we even need to patch clusters here? AFAIK we don't act on the contents 
of the config so having valid clusters is not necessary.


- Zameer Manji


On Jan. 8, 2015, 11:46 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 11:46 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Steve Niemitz

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

(Updated Jan. 8, 2015, 8:35 p.m.)


Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.


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


Repository: aurora


Description
---

This change adds support for launching docker containers through aurora.  These 
changes are based off of the discussion in 
https://issues.apache.org/jira/browse/AURORA-633

As of now, a special thermos_executor.sh script is needed to launch the 
executor inside docker containers.  A sample aurora file is in 
examples/jobs/docker.

In addition, mesos-slave must be run with `--containerizers=docker,mesos`, the 
example upstart config in examples/vagrant/upstart has been updated to reflect 
this.

More information is in subsequent review request comments.


Diffs (updated)
-

  Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
08ba1cdf88b712de22c26c04443079282db59ef9 
  docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
  docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
  examples/jobs/docker/hello_docker.aurora PRE-CREATION 
  examples/vagrant/aurorabuild.sh b7ea41719a8f41bb23d0254e732926d89399c77c 
  examples/vagrant/upstart/mesos-slave.conf 
512ce7ecf34042ed68dda55efb2dd0415f8469db 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
  src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
5226e3d1b303b1773a057078f2911c5ec2aa97f5 
  src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
d885b224ec5a1d529347d84e03ba98ab6734a126 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
  src/main/python/apache/aurora/config/thrift.py 
ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
  src/main/python/apache/aurora/executor/aurora_executor.py 
636b23d30a897b557eb8c3f8733c90b23cb807ef 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
  src/main/python/apache/aurora/executor/common/sandbox.py 
f47a32b3fefb4a89940b1ddc473b8316ac00df12 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
5e4bd65537d186459003c0b9434f1b769e04f448 
  src/main/python/apache/thermos/config/schema_base.py 
f9143cc1b83143d6147f59d90c79435d055d0518 
  src/main/python/apache/thermos/core/runner.py 
8aac6b50c66080abbb5308b367e9f74c487f42e3 
  src/main/resources/scheduler/assets/configSummary.html 
28878908b0c9381e366b71a3135dfc28c542a890 
  src/main/resources/scheduler/assets/js/services.js 
b744f375411e09b7f776e4a05ee5961227143439 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
5e54364a49a208bd5f19b9649633dc8feca591e9 
  src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
876e173ccbac04e4a06a245648c7c6af15eaaa92 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
ddcb511d108220ab5e4efcf3496458f7ab4a20c2 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
503e62f4cac872b14f6985b5bccc3e4dfcf81789 

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


Testing
---


Thanks,

Steve Niemitz



Re: Review Request 29698: Simplify client help output and solely use argparse.

2015-01-08 Thread Joshua Cohen

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

Ship it!



docs/client-commands.md
https://reviews.apache.org/r/29698/#comment111288

The cron commands are implemented now, right? Can we just kill these lines?


- Joshua Cohen


On Jan. 8, 2015, 8:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 8:32 p.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   docs/client-commands.md 75e69541fd95dfd9a7aa1e04de1a590b8fcbeacf 
   docs/client.md 3ec39b4f3bd6b45692aa1291e66a0a171d7dbb68 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/BUILD 
 bbac5c8efc9892fd2a966a6ac25fe05ffd740733 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
  

Re: Review Request 29698: Simplify client help output and solely use argparse.

2015-01-08 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 8, 2015, 12:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 12:32 p.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   docs/client-commands.md 75e69541fd95dfd9a7aa1e04de1a590b8fcbeacf 
   docs/client.md 3ec39b4f3bd6b45692aa1291e66a0a171d7dbb68 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/BUILD 
 bbac5c8efc9892fd2a966a6ac25fe05ffd740733 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
 Fully specified job instance key, in
 CLUSTER/ROLE/ENV/NAME[/INSTANCES] format. If INSTANCES
  

Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 8:03 p.m., Zameer Manji wrote:
  src/test/python/apache/aurora/client/cli/test_inspect.py, line 98
  https://reviews.apache.org/r/29696/diff/2/?file=813436#file813436line98
 
  I'm confused, if we define the print function here aren't we just 
  testing that our print function that we define here prints what we think it 
  does?

There's more code being tested than just the print function, but this mock 
serves to capture the output that a user would see.  (Well, sort of - this mock 
unfortunately includes logic.)


 On Jan. 8, 2015, 8:03 p.m., Zameer Manji wrote:
  src/test/python/apache/aurora/client/cli/test_inspect.py, line 120
  https://reviews.apache.org/r/29696/diff/2/?file=813436#file813436line120
 
  Do we even need to patch clusters here? AFAIK we don't act on the 
  contents of the config so having valid clusters is not necessary.

It's not needed, removed.


- Bill


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


On Jan. 8, 2015, 7:46 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 7:46 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Steve Niemitz


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 207
  https://reviews.apache.org/r/28920/diff/7/?file=808124#file808124line207
 
  What are the implications of this?  Does it mean a docker image must be 
  pre-loaded on the host for it to be used?  Can we cope with the user 
  specifying a bad path?  If not, what's the fallout - TASK_FAILED?
 
 Steve Niemitz wrote:
 The Volume options are for mounting paths on the host into the docker 
 container, and correspond to the -v flag of docker run 
 (https://docs.docker.com/userguide/dockervolumes/#mount-a-host-directory-as-a-data-volume).
   If the path is bad, it will fail but continue to run the container.  This 
 is goverened by mesos, and I actually have some plans to enhance their docker 
 integration.
 
 Bill Farner wrote:
 Awesome, please include that link here and in the .md.
 
 Question remains about whether the image must be pre-loaded on the 
 machine.

Will do.  I'll document it as well but the image doesn't need to be on the 
machine, Mesos will pull it if it doesn't exist.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 155
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line155
 
  From reading the code, it appears this additional path is needed for 
  them both to be available in the container.  If so, would it be easier to 
  just accept an arbitrary number of additional assets to copy into the 
  sandbox?  I would find that more generalized, and easier to understand.
  
  If you go with the above, i _think_ you can also safely nuke the extra 
  args plumbing.
 
 Steve Niemitz wrote:
 I think we'll still need extra args (its really useful right now to pass 
 ZK config in), but I like the idea of an arbitrary set of resources.  The 
 only trouble here would be figuring out which is the one to run and getting 
 the symlinks right.  Let's talk about this more.
 
 Bill Farner wrote:
 Wouldn't the extra args just be solved with the shim script?  I tell the 
 scheduler to copy `my_executor.sh` and `executor.pex` into the container, and 
 `my_executor.sh` contains the extra args.  I like this as a generalized 
 solution to augmenting default executor behavior, since it avoids feature 
 creep on our side just to save people the shim script.

Correct, it would.  My goal with this (which I'm happy to discuss more) was to 
make the wrapper script unnecessary in normal cases.  For us we just need to 
pass in the ZK config for the announcer, and I feel like this is the typical 
case.  Let me simmer on this one for a little bit.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java, line 109
  https://reviews.apache.org/r/28920/diff/7/?file=808132#file808132line109
 
  It's good form for this type of sanitization to happen here, but at 
  minimum the same sanitization must be done in `ConfigurationManager` to 
  give the user a good message and avoid accepting the bad config.
 
 Steve Niemitz wrote:
 I do a similar check in SchedulerMain.java, ~line 211.  Should I move the 
 check into ConfigurationManager?  ExecutorSettings.ctor checks as well.
 
 Bill Farner wrote:
 Doh, i left this comment in the wrong place.  This should have been a 
 request to sanitize the incoming thrift fields.

Ah that makes more sense!  I'll do so.


- Steve


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


On Jan. 8, 2015, 8:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Jan. 8, 2015, 8:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample aurora file is in 
 examples/jobs/docker.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 More information is in subsequent review request comments.
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 08ba1cdf88b712de22c26c04443079282db59ef9 
   

Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (4053924) 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 Jan. 8, 2015, 8:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Jan. 8, 2015, 8:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample aurora file is in 
 examples/jobs/docker.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 More information is in subsequent review request comments.
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 08ba1cdf88b712de22c26c04443079282db59ef9 
   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/vagrant/aurorabuild.sh b7ea41719a8f41bb23d0254e732926d89399c77c 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
   src/main/python/apache/aurora/executor/common/sandbox.py 
 f47a32b3fefb4a89940b1ddc473b8316ac00df12 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/config/schema_base.py 
 f9143cc1b83143d6147f59d90c79435d055d0518 
   src/main/python/apache/thermos/core/runner.py 
 8aac6b50c66080abbb5308b367e9f74c487f42e3 
   src/main/resources/scheduler/assets/configSummary.html 
 28878908b0c9381e366b71a3135dfc28c542a890 
   src/main/resources/scheduler/assets/js/services.js 
 b744f375411e09b7f776e4a05ee5961227143439 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 5e54364a49a208bd5f19b9649633dc8feca591e9 
   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
 876e173ccbac04e4a06a245648c7c6af15eaaa92 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 ddcb511d108220ab5e4efcf3496458f7ab4a20c2 
   src/test/python/apache/aurora/executor/test_thermos_executor.py 
 503e62f4cac872b14f6985b5bccc3e4dfcf81789 
 
 Diff: https://reviews.apache.org/r/28920/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 29698: Simplify client help output and solely use argparse.

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (4053924) 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 Jan. 8, 2015, 8:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 8:32 p.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   docs/client-commands.md 75e69541fd95dfd9a7aa1e04de1a590b8fcbeacf 
   docs/client.md 3ec39b4f3bd6b45692aa1291e66a0a171d7dbb68 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/BUILD 
 bbac5c8efc9892fd2a966a6ac25fe05ffd740733 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]
 

Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java, line 109
  https://reviews.apache.org/r/28920/diff/7/?file=808132#file808132line109
 
  It's good form for this type of sanitization to happen here, but at 
  minimum the same sanitization must be done in `ConfigurationManager` to 
  give the user a good message and avoid accepting the bad config.
 
 Steve Niemitz wrote:
 I do a similar check in SchedulerMain.java, ~line 211.  Should I move the 
 check into ConfigurationManager?  ExecutorSettings.ctor checks as well.

Doh, i left this comment in the wrong place.  This should have been a request 
to sanitize the incoming thrift fields.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 207
  https://reviews.apache.org/r/28920/diff/7/?file=808124#file808124line207
 
  What are the implications of this?  Does it mean a docker image must be 
  pre-loaded on the host for it to be used?  Can we cope with the user 
  specifying a bad path?  If not, what's the fallout - TASK_FAILED?
 
 Steve Niemitz wrote:
 The Volume options are for mounting paths on the host into the docker 
 container, and correspond to the -v flag of docker run 
 (https://docs.docker.com/userguide/dockervolumes/#mount-a-host-directory-as-a-data-volume).
   If the path is bad, it will fail but continue to run the container.  This 
 is goverened by mesos, and I actually have some plans to enhance their docker 
 integration.

Awesome, please include that link here and in the .md.

Question remains about whether the image must be pre-loaded on the machine.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 215
  https://reviews.apache.org/r/28920/diff/7/?file=808124#file808124line215
 
  Perhaps this should be called `imageName`?  Without help in the name, i 
  could be convinced this was a path or URI.
  
  That said, what is this for?  As a user, can i incorrectly 
  specify/format this string, or is it for my own purposes?
 
 Steve Niemitz wrote:
 I tried to avoid a name with name in it to avoid confusion with other 
 name fields that are purely for ID reasons (ok that sentence was a 
 mouthful).  I can enhance the docs though to be more clear that it expects a 
 docker image name and not a URI/etc.
 
 This field is actually the docker container (image) that will be run.

Ah, in that case i agree.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 153
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line153
 
  s/aurora/mesos/?  IIUC it's the slave that does the copy.
 
 Steve Niemitz wrote:
 how about s/aurora will/aurora will configure mesos to/?

+1


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 155
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line155
 
  From reading the code, it appears this additional path is needed for 
  them both to be available in the container.  If so, would it be easier to 
  just accept an arbitrary number of additional assets to copy into the 
  sandbox?  I would find that more generalized, and easier to understand.
  
  If you go with the above, i _think_ you can also safely nuke the extra 
  args plumbing.
 
 Steve Niemitz wrote:
 I think we'll still need extra args (its really useful right now to pass 
 ZK config in), but I like the idea of an arbitrary set of resources.  The 
 only trouble here would be figuring out which is the one to run and getting 
 the symlinks right.  Let's talk about this more.

Wouldn't the extra args just be solved with the shim script?  I tell the 
scheduler to copy `my_executor.sh` and `executor.pex` into the container, and 
`my_executor.sh` contains the extra args.  I like this as a generalized 
solution to augmenting default executor behavior, since it avoids feature creep 
on our side just to save people the shim script.


- Bill


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


On Jan. 8, 2015, 8:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Jan. 8, 2015, 8:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 

Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Bill Farner

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

(Updated Jan. 8, 2015, 9:32 p.m.)


Review request for Aurora, Kevin Sweeney and Zameer Manji.


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


Repository: aurora


Description
---

In the course of this, i took a stab at cleaning up the unit test to remove 
unnecessary mocking and avoid writing to a temporary file.  The nice outcome of 
the mock removal is that the job configuration in our code is identical to what 
would appear in a configuration file.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/__init__.py 
395819fdf24b7919b32be51060fb5b581c8e1514 
  src/main/python/apache/aurora/client/cli/jobs.py 
744d53405081e75b9aedcdcb7e4a823a2e0e743f 
  src/test/python/apache/aurora/client/cli/test_inspect.py 
1c07d912c95eafa292d5451f4d6c72e9a711dae0 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 8, 2015, 1:32 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 1:32 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Jan. 8, 2015, 7:46 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 7:46 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 6:15 p.m., Maxim Khutornenko wrote:
  src/main/python/apache/aurora/client/cli/task.py, line 46
  https://reviews.apache.org/r/29698/diff/1/?file=811543#file811543line46
 
  The majority of subcommand help strings (i.e. jobs.py) start with 
  capital letter. Suggest making it consistent here.

Thanks, you caught me trying to hunt for the convention.  Fixed.


- Bill


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


On Jan. 8, 2015, 3:33 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 3:33 a.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   CLUSTER/ROLE/ENV/NAME[/INSTANCES]

Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 6:50 p.m., Kevin Sweeney wrote:
  src/main/python/apache/aurora/client/cli/task.py, line 83
  https://reviews.apache.org/r/29698/diff/1/?file=811543#file811543line83
 
  since these aren't multiline anymore you don't need the triple quotes. 
  In fact, you can do
  
  ```py
  help = ...
  ```
  
  instead of @property.

Good catch on both, changed throughout.


- Bill


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


On Jan. 8, 2015, 3:33 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 3:33 a.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine that a task instance is
   running on.
 
 
 vagrant@192:~$ aurora task run -h
 usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
[--executor-sandbox] [--verbose]
[--skip-hooks hook,hook,...]
CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line
 
 positional arguments:
   

Re: Review Request 29698: Simplify client help output to solely use argparse.

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 6:50 p.m., Kevin Sweeney wrote:
  src/main/python/apache/aurora/client/cli/task.py, line 83
  https://reviews.apache.org/r/29698/diff/1/?file=811543#file811543line83
 
  since these aren't multiline anymore you don't need the triple quotes. 
  In fact, you can do
  
  ```py
  help = ...
  ```
  
  instead of @property.
 
 Bill Farner wrote:
 Good catch on both, changed throughout.

Scratch that - this causes checkstyle to bark with
```
T001:ERROR   src/main/python/apache/aurora/client/cli/jobs.py:402 Class globals 
must be UPPER_SNAKE_CASED
 |  help = Open a job's scheduler page in the web browser.
```

Punting to you if you would like to follow up.  Personally, i'm happy to leave 
this alone since it will disappear when we remove the class hierarchy here.


- Bill


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


On Jan. 8, 2015, 3:33 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29698/
 ---
 
 (Updated Jan. 8, 2015, 3:33 a.m.)
 
 
 Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
 
 
 Bugs: AURORA-994
 https://issues.apache.org/jira/browse/AURORA-994
 
 
 Repository: aurora
 
 
 Description
 ---
 
 The only downside with this patch is that we've technically lost test 
 coverage of our help output.  This is rather involved if we want to change 
 it.  I ventured down the path of preserving `test_help.py`, but the best i 
 could come up with (without a larger refactor on our end) was to patch 
 `_print_message` and `exit` functions from `argparser.ArgumentParser`.  This 
 still did not address the fact that it accesses `sys.argv[0]` directly.  
 Again - we could restructure to work around it, but at this point i think the 
 value is dubious.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/client.py 
 939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
   src/main/python/apache/aurora/client/cli/options.py 
 b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
   src/main/python/apache/aurora/client/cli/task.py 
 e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
   src/test/python/apache/aurora/client/cli/test_help.py 
 9fa05e683f01a0e51253e08aa7fba69fd49d3756 
   src/test/python/apache/aurora/client/cli/test_plugins.py 
 cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
   src/test/python/apache/aurora/client/cli/util.py 
 1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 
 
 Diff: https://reviews.apache.org/r/29698/diff/
 
 
 Testing
 ---
 
 In vagrant:
 ```
 vagrant@192:~$ aurora
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora -h
 usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...
 
 optional arguments:
   -h, --helpshow this help message and exit
 
 commands:
   {task,quota,cron,job,config,sla,beta-update}
 taskWork with a task running in an Apache Aurora cluster
 quota   Work with quota settings for an Apache Aurora cluster
 cronWork with entries in the aurora cron scheduler
 job Work with an aurora job
 config  Work with an aurora configuration file
 sla Work with SLA data in Aurora cluster.
 beta-update Interact with the aurora update service.
 
 
 vagrant@192:~$ aurora task
 usage: aurora task [-h] {run,ssh} ...
 aurora task: error: too few arguments
 
 
 vagrant@192:~$ aurora task -h
 usage: aurora task [-h] {run,ssh} ...
 
 optional arguments:
   -h, --help  show this help message and exit
 
 subcommands:
   {run,ssh}
 run   runs a shell command on machines currently hosting instances of
   a single job. This feature supports the same command line
   wildcards that are used to populate a job's commands. This means
   anything in the {{mesos.*}} and {{thermos.*}} namespaces.
 ssh   initiates an SSH session on the machine 

Re: Review Request 29698: Simplify client help output and solely use argparse.

2015-01-08 Thread Bill Farner

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

(Updated Jan. 8, 2015, 8:26 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

In the updated patch, i've removed docs/client.md.  It served as more of a 
design doc for the new client, and no longer serves as useful documentation.


Summary (updated)
-

Simplify client help output and solely use argparse.


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


Repository: aurora


Description
---

The only downside with this patch is that we've technically lost test coverage 
of our help output.  This is rather involved if we want to change it.  I 
ventured down the path of preserving `test_help.py`, but the best i could come 
up with (without a larger refactor on our end) was to patch `_print_message` 
and `exit` functions from `argparser.ArgumentParser`.  This still did not 
address the fact that it accesses `sys.argv[0]` directly.  Again - we could 
restructure to work around it, but at this point i think the value is dubious.


Diffs (updated)
-

  docs/client-commands.md 75e69541fd95dfd9a7aa1e04de1a590b8fcbeacf 
  docs/client.md 3ec39b4f3bd6b45692aa1291e66a0a171d7dbb68 
  src/main/python/apache/aurora/client/cli/__init__.py 
395819fdf24b7919b32be51060fb5b581c8e1514 
  src/main/python/apache/aurora/client/cli/client.py 
939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
  src/main/python/apache/aurora/client/cli/options.py 
b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
  src/main/python/apache/aurora/client/cli/task.py 
e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
  src/test/python/apache/aurora/client/cli/test_help.py 
9fa05e683f01a0e51253e08aa7fba69fd49d3756 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
  src/test/python/apache/aurora/client/cli/util.py 
1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 

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


Testing
---

In vagrant:
```
vagrant@192:~$ aurora
usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...

optional arguments:
  -h, --helpshow this help message and exit

commands:
  {task,quota,cron,job,config,sla,beta-update}
taskWork with a task running in an Apache Aurora cluster
quota   Work with quota settings for an Apache Aurora cluster
cronWork with entries in the aurora cron scheduler
job Work with an aurora job
config  Work with an aurora configuration file
sla Work with SLA data in Aurora cluster.
beta-update Interact with the aurora update service.


vagrant@192:~$ aurora -h
usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...

optional arguments:
  -h, --helpshow this help message and exit

commands:
  {task,quota,cron,job,config,sla,beta-update}
taskWork with a task running in an Apache Aurora cluster
quota   Work with quota settings for an Apache Aurora cluster
cronWork with entries in the aurora cron scheduler
job Work with an aurora job
config  Work with an aurora configuration file
sla Work with SLA data in Aurora cluster.
beta-update Interact with the aurora update service.


vagrant@192:~$ aurora task
usage: aurora task [-h] {run,ssh} ...
aurora task: error: too few arguments


vagrant@192:~$ aurora task -h
usage: aurora task [-h] {run,ssh} ...

optional arguments:
  -h, --help  show this help message and exit

subcommands:
  {run,ssh}
run   runs a shell command on machines currently hosting instances of
  a single job. This feature supports the same command line
  wildcards that are used to populate a job's commands. This means
  anything in the {{mesos.*}} and {{thermos.*}} namespaces.
ssh   initiates an SSH session on the machine that a task instance is
  running on.


vagrant@192:~$ aurora task run -h
usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
   [--executor-sandbox] [--verbose]
   [--skip-hooks hook,hook,...]
   CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line

positional arguments:
  CLUSTER/ROLE/ENV/NAME[/INSTANCES]
Fully specified job instance key, in
CLUSTER/ROLE/ENV/NAME[/INSTANCES] format. If INSTANCES
is omitted, then all instances will be operated on.
  unix_command_line

optional arguments:
  -h, --helpshow this help message and exit
  --threads NUM_THREADS, -t NUM_THREADS
Number of threads to use
  --ssh-user 

Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 2:58 a.m., Joshua Cohen wrote:
  src/test/python/apache/aurora/client/cli/test_inspect.py, line 122
  https://reviews.apache.org/r/29696/diff/1/?file=810573#file810573line122
 
  Don't leave me hanging... it would be nice to what?!? ;)

Hah!  Thanks, fixed.


- Bill


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


On Jan. 8, 2015, 2:19 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 2:19 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Bill Farner


 On Jan. 8, 2015, 5:55 p.m., Maxim Khutornenko wrote:
  src/test/python/apache/aurora/client/cli/test_inspect.py, line 126
  https://reviews.apache.org/r/29696/diff/1/?file=810573#file810573line126
 
  Is it a copy-paste from somewhere? I don't see anything using waiting 
  events around inspect command.

Yeah, rampant copy-pasting going on.  Thanks for catching.  This is a small 
part of why using patch() is a pain, makes it hard to tell when the patch is 
unnecessary.

Removed htis and others.


- Bill


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


On Jan. 8, 2015, 2:19 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 2:19 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Bill Farner

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

(Updated Jan. 8, 2015, 7:46 p.m.)


Review request for Aurora, Kevin Sweeney and Zameer Manji.


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


Repository: aurora


Description
---

In the course of this, i took a stab at cleaning up the unit test to remove 
unnecessary mocking and avoid writing to a temporary file.  The nice outcome of 
the mock removal is that the job configuration in our code is identical to what 
would appear in a configuration file.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/__init__.py 
395819fdf24b7919b32be51060fb5b581c8e1514 
  src/main/python/apache/aurora/client/cli/jobs.py 
744d53405081e75b9aedcdcb7e4a823a2e0e743f 
  src/test/python/apache/aurora/client/cli/test_inspect.py 
1c07d912c95eafa292d5451f4d6c72e9a711dae0 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 29696: Fix bad call to print_out causing 'job inspect --raw' to fail.

2015-01-08 Thread Aurora ReviewBot

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

Ship it!


Master (4053924) 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 Jan. 8, 2015, 7:46 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29696/
 ---
 
 (Updated Jan. 8, 2015, 7:46 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Bugs: AURORA-990
 https://issues.apache.org/jira/browse/AURORA-990
 
 
 Repository: aurora
 
 
 Description
 ---
 
 In the course of this, i took a stab at cleaning up the unit test to remove 
 unnecessary mocking and avoid writing to a temporary file.  The nice outcome 
 of the mock removal is that the job configuration in our code is identical to 
 what would appear in a configuration file.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 395819fdf24b7919b32be51060fb5b581c8e1514 
   src/main/python/apache/aurora/client/cli/jobs.py 
 744d53405081e75b9aedcdcb7e4a823a2e0e743f 
   src/test/python/apache/aurora/client/cli/test_inspect.py 
 1c07d912c95eafa292d5451f4d6c72e9a711dae0 
 
 Diff: https://reviews.apache.org/r/29696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 29698: Simplify client help output and solely use argparse.

2015-01-08 Thread Bill Farner

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

(Updated Jan. 8, 2015, 8:32 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

Fixed build, for some reason reviewbot passed the previous patch even though a 
BUILD file was in need of update.


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


Repository: aurora


Description
---

The only downside with this patch is that we've technically lost test coverage 
of our help output.  This is rather involved if we want to change it.  I 
ventured down the path of preserving `test_help.py`, but the best i could come 
up with (without a larger refactor on our end) was to patch `_print_message` 
and `exit` functions from `argparser.ArgumentParser`.  This still did not 
address the fact that it accesses `sys.argv[0]` directly.  Again - we could 
restructure to work around it, but at this point i think the value is dubious.


Diffs (updated)
-

  docs/client-commands.md 75e69541fd95dfd9a7aa1e04de1a590b8fcbeacf 
  docs/client.md 3ec39b4f3bd6b45692aa1291e66a0a171d7dbb68 
  src/main/python/apache/aurora/client/cli/__init__.py 
395819fdf24b7919b32be51060fb5b581c8e1514 
  src/main/python/apache/aurora/client/cli/client.py 
939e32b0287a4a6e9cd66c4d6ffe05b32ed26d78 
  src/main/python/apache/aurora/client/cli/options.py 
b7f5a031d135a33ec2d79aa521ce9c1438eb58c1 
  src/main/python/apache/aurora/client/cli/task.py 
e084c5bef54d8a726276764ed7e5ce44cdc99ec5 
  src/test/python/apache/aurora/client/cli/BUILD 
bbac5c8efc9892fd2a966a6ac25fe05ffd740733 
  src/test/python/apache/aurora/client/cli/test_help.py 
9fa05e683f01a0e51253e08aa7fba69fd49d3756 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
cf742a3feb12c6bb8fc6e80f15daaac7c2b2bf55 
  src/test/python/apache/aurora/client/cli/util.py 
1fa1207d9380e57ac77d2aa24725b9ac39c83d4c 

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


Testing
---

In vagrant:
```
vagrant@192:~$ aurora
usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...

optional arguments:
  -h, --helpshow this help message and exit

commands:
  {task,quota,cron,job,config,sla,beta-update}
taskWork with a task running in an Apache Aurora cluster
quota   Work with quota settings for an Apache Aurora cluster
cronWork with entries in the aurora cron scheduler
job Work with an aurora job
config  Work with an aurora configuration file
sla Work with SLA data in Aurora cluster.
beta-update Interact with the aurora update service.


vagrant@192:~$ aurora -h
usage: aurora [-h] {task,quota,cron,job,config,sla,beta-update} ...

optional arguments:
  -h, --helpshow this help message and exit

commands:
  {task,quota,cron,job,config,sla,beta-update}
taskWork with a task running in an Apache Aurora cluster
quota   Work with quota settings for an Apache Aurora cluster
cronWork with entries in the aurora cron scheduler
job Work with an aurora job
config  Work with an aurora configuration file
sla Work with SLA data in Aurora cluster.
beta-update Interact with the aurora update service.


vagrant@192:~$ aurora task
usage: aurora task [-h] {run,ssh} ...
aurora task: error: too few arguments


vagrant@192:~$ aurora task -h
usage: aurora task [-h] {run,ssh} ...

optional arguments:
  -h, --help  show this help message and exit

subcommands:
  {run,ssh}
run   runs a shell command on machines currently hosting instances of
  a single job. This feature supports the same command line
  wildcards that are used to populate a job's commands. This means
  anything in the {{mesos.*}} and {{thermos.*}} namespaces.
ssh   initiates an SSH session on the machine that a task instance is
  running on.


vagrant@192:~$ aurora task run -h
usage: aurora task run [-h] [--threads NUM_THREADS] [--ssh-user ssh_username]
   [--executor-sandbox] [--verbose]
   [--skip-hooks hook,hook,...]
   CLUSTER/ROLE/ENV/NAME[/INSTANCES] unix_command_line

positional arguments:
  CLUSTER/ROLE/ENV/NAME[/INSTANCES]
Fully specified job instance key, in
CLUSTER/ROLE/ENV/NAME[/INSTANCES] format. If INSTANCES
is omitted, then all instances will be operated on.
  unix_command_line

optional arguments:
  -h, --helpshow this help message and exit
  --threads NUM_THREADS, -t NUM_THREADS
Number of threads to use
  --ssh-user ssh_username, -l ssh_username
   

Re: Review Request 28920: Add support for docker containers to aurora

2015-01-08 Thread Steve Niemitz


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 207
  https://reviews.apache.org/r/28920/diff/7/?file=808124#file808124line207
 
  What are the implications of this?  Does it mean a docker image must be 
  pre-loaded on the host for it to be used?  Can we cope with the user 
  specifying a bad path?  If not, what's the fallout - TASK_FAILED?

The Volume options are for mounting paths on the host into the docker 
container, and correspond to the -v flag of docker run 
(https://docs.docker.com/userguide/dockervolumes/#mount-a-host-directory-as-a-data-volume).
  If the path is bad, it will fail but continue to run the container.  This is 
goverened by mesos, and I actually have some plans to enhance their docker 
integration.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 215
  https://reviews.apache.org/r/28920/diff/7/?file=808124#file808124line215
 
  Perhaps this should be called `imageName`?  Without help in the name, i 
  could be convinced this was a path or URI.
  
  That said, what is this for?  As a user, can i incorrectly 
  specify/format this string, or is it for my own purposes?

I tried to avoid a name with name in it to avoid confusion with other name 
fields that are purely for ID reasons (ok that sentence was a mouthful).  I can 
enhance the docs though to be more clear that it expects a docker image name 
and not a URI/etc.

This field is actually the docker container (image) that will be run.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 148
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line148
 
  It would be really useful to see a working example of these 
  configuration parameters in concert.  I suggest you go ahead and wire it up 
  in the upstart configs we have, and link to them from this doc.
  
  Taking it a step further, it would be awesome if this was exercised in 
  `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`.

Totally agree, I actually have a jira ticket (in our own jira) to do just this. 
 I assume you mean the vagrant upstart configs?


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 153
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line153
 
  s/aurora/mesos/?  IIUC it's the slave that does the copy.

how about s/aurora will/aurora will configure mesos to/?


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  docs/deploying-aurora-scheduler.md, line 155
  https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line155
 
  From reading the code, it appears this additional path is needed for 
  them both to be available in the container.  If so, would it be easier to 
  just accept an arbitrary number of additional assets to copy into the 
  sandbox?  I would find that more generalized, and easier to understand.
  
  If you go with the above, i _think_ you can also safely nuke the extra 
  args plumbing.

I think we'll still need extra args (its really useful right now to pass ZK 
config in), but I like the idea of an arbitrary set of resources.  The only 
trouble here would be figuring out which is the one to run and getting the 
symlinks right.  Let's talk about this more.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java, line 108
  https://reviews.apache.org/r/28920/diff/7/?file=808132#file808132line108
 
  I applied your patch and removed these PMD exclusions without any 
  issue.  Are they needed?
  
  As a general practice - we avoid decorating the code with hints to code 
  quality checkers.  This could vary from making the code appease the checker 
  to disabling the rule.

Ah, so this used to be a != null check that PMD complained about, but since I 
refactored it recently to use Optional PMD stopped complaining.  I'll remove 
these hits.


 On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java, line 109
  https://reviews.apache.org/r/28920/diff/7/?file=808132#file808132line109
 
  It's good form for this type of sanitization to happen here, but at 
  minimum the same sanitization must be done in `ConfigurationManager` to 
  give the user a good message and avoid accepting the bad config.

I do a similar check in SchedulerMain.java, ~line 211.  Should I move the check 
into ConfigurationManager?  ExecutorSettings.ctor checks as well.


- Steve


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


On Jan. 6, 2015, 11:32 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically