Re: Review Request 29731: Service status endpoint.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
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.
--- 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.
--- 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
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.
--- 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.
--- 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.
--- 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
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.
--- 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
--- 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.
--- 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.
--- 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.
--- 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.
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.
--- 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.
--- 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.
--- 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
--- 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.
--- 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.
--- 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
--- 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.
--- 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.
--- 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.
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
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
--- 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.
--- 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
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.
--- 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.
--- 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.
--- 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.
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.
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.
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.
--- 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.
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.
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.
--- 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.
--- 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.
--- 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
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