Review Request 25297: Make config-file an optional parameter for job restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/ --- Review request for Aurora. Repository: aurora Description --- Make config-file an optional parameter for job restart. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 Diff: https://reviews.apache.org/r/25297/diff/ Testing --- - Ran all unit tests successfully. - Ran end-to-end tests successfully. - Thanks, Mark Chu-Carroll
Re: Review Request 25297: Make config-file an optional parameter for job restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/ --- (Updated Sept. 3, 2014, 10:06 a.m.) Review request for Aurora. Bugs: aurora-673 https://issues.apache.org/jira/browse/aurora-673 Repository: aurora Description --- Make config-file an optional parameter for job restart. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 Diff: https://reviews.apache.org/r/25297/diff/ Testing --- - Ran all unit tests successfully. - Ran end-to-end tests successfully. - Thanks, Mark Chu-Carroll
Re: Review Request 25297: Make config-file an optional parameter for job restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/ --- (Updated Sept. 3, 2014, 10:07 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-673 https://issues.apache.org/jira/browse/aurora-673 Repository: aurora Description --- Make config-file an optional parameter for job restart. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 Diff: https://reviews.apache.org/r/25297/diff/ Testing --- - Ran all unit tests successfully. - Ran end-to-end tests successfully. - Thanks, Mark Chu-Carroll
Review Request 25300: Add a state machine to react to job update status changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This will be used by the job updater to gate insertion of JobUpdateEvents. I chose not to use StateMachine here as it actually added complexity - in this case we don't benefit from transition callbacks, and the result is based only on the target state (rather than the transition). Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner
Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.
On Sept. 3, 2014, 1:40 a.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, line 175 https://reviews.apache.org/r/25257/diff/2/?file=674608#file674608line175 Can this TODO be dropped now? Sure, done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25257/#review52118 --- On Sept. 2, 2014, 11:39 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25257/ --- (Updated Sept. 2, 2014, 11:39 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji. Bugs: AURORA-658 https://issues.apache.org/jira/browse/AURORA-658 Repository: aurora Description --- The big improvement over the previous incantation of local scheduler mode is that arguments like `testing_isolated_scheduler` don't leak into production builds. There is also less affordance made in SchedulerMain and modules for testing mode - the behavior changes with modules rather than branches. This could be extended pretty easily to offer more faked behavior, but i stopped at providing an offer loop. With this, jobs can be submitted, and show as moving to RUNNING. As mentioned in a TODO, i would like to change SchedulerIT in a follow-up to use the same approach. Diffs - build.gradle 66c8205c8354048c398623edf7ba75e721376b01 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java ec31c49da55b68e89bf13f08f4bb6f571a46fbc3 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7178a924ef8d7966bf24fa96657ea514080b1d00 src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java PRE-CREATION Diff: https://reviews.apache.org/r/25257/diff/ Testing --- ./gradlew run, also ran directly in intellij Thanks, Bill Farner
Re: Review Request 25206: Fix aurora run in end to end tests.
On Sept. 3, 2014, 1:47 a.m., Joe Smith wrote: src/test/sh/org/apache/aurora/e2e/test_run.sh, line 23 https://reviews.apache.org/r/25206/diff/1/?file=672554#file672554line23 aurora job run? Joe Smith wrote: Kevin and Toby, can you two take a look at this ASAP? I'd like to have the e2e tests fixed before I ship https://reviews.apache.org/r/25208/ It defaults to 'aurora run', for the aurora1 client, but we pass in 'aurora2 job run' from the test_end_to_end_v2.sh script. Are you saying to switch the default (or maybe I should just remove the default altogether)? - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52121 --- On Aug. 29, 2014, 11:17 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Aug. 29, 2014, 11:17 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25297: Make config-file an optional parameter for job restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/#review52183 --- src/test/python/apache/aurora/client/cli/test_restart.py https://reviews.apache.org/r/25297/#comment90939 You've got comments on these asserts in the test above, but they're not replicated here. If that test were to change the reason for these numbers would be lost to the ages (or at least git history). How do you feel about pulling these asserts out to a separate function that's reused by both and documenting the reasoning there? src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh https://reviews.apache.org/r/25297/#comment90940 Kill trailing whitespace? - Joshua Cohen On Sept. 3, 2014, 2:07 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/ --- (Updated Sept. 3, 2014, 2:07 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-673 https://issues.apache.org/jira/browse/aurora-673 Repository: aurora Description --- Make config-file an optional parameter for job restart. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 Diff: https://reviews.apache.org/r/25297/diff/ Testing --- - Ran all unit tests successfully. - Ran end-to-end tests successfully. - Thanks, Mark Chu-Carroll
Re: Review Request 25285: Upgrade to latest in jetty 7.x series.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/#review52186 --- Ship it! Ship It! - Joshua Cohen On Sept. 3, 2014, 5:23 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/ --- (Updated Sept. 3, 2014, 5:23 a.m.) Review request for Aurora, David McLaughlin and Kevin Sweeney. Bugs: AURORA-679 https://issues.apache.org/jira/browse/AURORA-679 Repository: aurora Description --- Long story short: this change adds code, but enables us to remove code in a follow-up. This change brings us to Jetty 7, which is the newest jetty that still uses servlet-api 2.5. Jetty 8/9 will require a larger yak shave since it pulls a dependency thread all the way to guice (servlet-api - jersey-guice - guice-servlet - guice). This is also a small step towards insulating ourselves from transitive dependencies pulled in from twitter commons. [1] http://wiki.eclipse.org/Jetty/Starting/Porting_to_Jetty_7 Diffs - build.gradle 66c8205c8354048c398623edf7ba75e721376b01 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/RequestLoggerTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25285/diff/ Testing --- ./gradlew build -Pq Ran the scheduler in vagrant, verified index page works, request logging works, and all pages linked from the index page work. Thanks, Bill Farner
Re: Review Request 25285: Upgrade to latest in jetty 7.x series.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/#review52190 --- Ship it! src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java https://reviews.apache.org/r/25285/#comment90944 typo - Maxim Khutornenko On Sept. 3, 2014, 5:23 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/ --- (Updated Sept. 3, 2014, 5:23 a.m.) Review request for Aurora, David McLaughlin and Kevin Sweeney. Bugs: AURORA-679 https://issues.apache.org/jira/browse/AURORA-679 Repository: aurora Description --- Long story short: this change adds code, but enables us to remove code in a follow-up. This change brings us to Jetty 7, which is the newest jetty that still uses servlet-api 2.5. Jetty 8/9 will require a larger yak shave since it pulls a dependency thread all the way to guice (servlet-api - jersey-guice - guice-servlet - guice). This is also a small step towards insulating ourselves from transitive dependencies pulled in from twitter commons. [1] http://wiki.eclipse.org/Jetty/Starting/Porting_to_Jetty_7 Diffs - build.gradle 66c8205c8354048c398623edf7ba75e721376b01 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/RequestLoggerTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25285/diff/ Testing --- ./gradlew build -Pq Ran the scheduler in vagrant, verified index page works, request logging works, and all pages linked from the index page work. Thanks, Bill Farner
Re: Review Request 25208: Increment Mesos version to 0.20.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25208/#review52192 --- Ship it! Ship It! - Brian Wickman On Sept. 3, 2014, 4:06 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25208/ --- (Updated Sept. 3, 2014, 4:06 a.m.) Review request for Aurora, Kevin Sweeney, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-674 https://issues.apache.org/jira/browse/AURORA-674 Repository: aurora Description --- There's an API change when moving the python executor to 0.20.0, which should deserve extra scrutiny. Diffs - 3rdparty/python/BUILD edc446e6d76d53a4c10a703cf6e84e29b8d42366 Vagrantfile c8cb2d440330780c10eeff47d46cc1a3f1aa4910 build.gradle 66c8205c8354048c398623edf7ba75e721376b01 examples/vagrant/provision-dev-cluster.sh 2ce74f8a6dc663b62d254bc9d3a5c06cb412107f src/main/python/apache/aurora/executor/BUILD c1c50dbcdcaa56f8ff970ceac59d5db537e1d048 src/main/python/apache/aurora/executor/aurora_executor.py 8985aeb60e2c890113ea7d25039cdab5b7c9a19e src/main/python/apache/aurora/executor/bin/BUILD 93517150db96600c92ea25c0de29ec20736d72f7 src/main/python/apache/aurora/executor/bin/gc_executor_main.py bc20dad1e5434a98fb51ab052efa8ced760e74e0 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py aacc19a3bd272b08dee8a77d7a64490d0fcad3fa src/main/python/apache/aurora/executor/common/BUILD 0a4d35afcbfca8730d8bb7373795f9b3b6a74fd7 src/main/python/apache/aurora/executor/common/health_checker.py 671f4970d4637b8cb68ec89e5dfe96a2f4cd077b src/main/python/apache/aurora/executor/common/kill_manager.py 6e482cfa7d2e081804fe6992527bae0ac5b65080 src/main/python/apache/aurora/executor/common/status_checker.py eda30e38d34adc5c78b828d0aea31f0721bf9b20 src/main/python/apache/aurora/executor/executor_base.py c632b77d972f76f3e260bb7413aa6100cda46607 src/main/python/apache/aurora/executor/gc_executor.py c45d5eb9a99e310e0716abd626ca34615f63b0ae src/main/python/apache/aurora/executor/thermos_task_runner.py 03a40e63573322f2d40e59a8ce84e9b9a1b90aa6 src/test/python/apache/aurora/executor/BUILD 3152ec0c0e7dfaec5d756542a89243756b27725d src/test/python/apache/aurora/executor/common/BUILD 6316c7fc724c8e2245f5ce1a93c4a2daecd9f26b src/test/python/apache/aurora/executor/common/test_executor_timeout.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py baeaba29e7679203d83f6ea81455973baf07f324 src/test/python/apache/aurora/executor/common/test_kill_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_status_checker.py 6ca4fd469bdc7cd21033ccdea460292463bc1cf8 src/test/python/apache/aurora/executor/test_executor_base.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 0b2278a94667ca4d43e8a84649ed5312acacf7bb src/test/python/apache/aurora/executor/test_status_manager.py PRE-CREATION src/test/python/apache/aurora/executor/test_thermos_executor.py 55747a19478572e39d1f00f799727d3c3db174c9 src/test/python/apache/aurora/executor/test_thermos_task_runner.py f7af70fda8fef91018ff9d10dc1db6d43fd16ac7 Diff: https://reviews.apache.org/r/25208/diff/ Testing --- $ ./gradlew test [tw-mbp13-jsmith aurora (yasumoto/mesos_0.20.0)]$ ./build-support/python/checkstyle-check ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS e2e (not complete until
Re: Review Request 25300: Add a state machine to react to job update status changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/#review52193 --- Ship it! Ship It! - Joshua Cohen On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/ --- (Updated Sept. 3, 2014, 4:02 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This will be used by the job updater to gate insertion of JobUpdateEvents. I chose not to use StateMachine here as it actually added complexity - in this case we don't benefit from transition callbacks, and the result is based only on the target state (rather than the transition). Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner
Re: Review Request 25300: Add a state machine to react to job update status changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/#review52195 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java https://reviews.apache.org/r/25300/#comment90946 This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED - ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift APIs (i.e. pause/resume/abort). Would it make sense to alter ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if from != to? - Maxim Khutornenko On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/ --- (Updated Sept. 3, 2014, 4:02 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This will be used by the job updater to gate insertion of JobUpdateEvents. I chose not to use StateMachine here as it actually added complexity - in this case we don't benefit from transition callbacks, and the result is based only on the target state (rather than the transition). Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner
Re: Review Request 25208: Increment Mesos version to 0.20.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25208/#review52198 --- Ship it! src/main/python/apache/aurora/executor/thermos_task_runner.py https://reviews.apache.org/r/25208/#comment90949 mesos_pb2 looks a bit cryptic here. How about the approach you sued elsewhere: from mesos.interface.mesos_pb2 import TaskState - Maxim Khutornenko On Sept. 3, 2014, 4:06 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25208/ --- (Updated Sept. 3, 2014, 4:06 a.m.) Review request for Aurora, Kevin Sweeney, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-674 https://issues.apache.org/jira/browse/AURORA-674 Repository: aurora Description --- There's an API change when moving the python executor to 0.20.0, which should deserve extra scrutiny. Diffs - 3rdparty/python/BUILD edc446e6d76d53a4c10a703cf6e84e29b8d42366 Vagrantfile c8cb2d440330780c10eeff47d46cc1a3f1aa4910 build.gradle 66c8205c8354048c398623edf7ba75e721376b01 examples/vagrant/provision-dev-cluster.sh 2ce74f8a6dc663b62d254bc9d3a5c06cb412107f src/main/python/apache/aurora/executor/BUILD c1c50dbcdcaa56f8ff970ceac59d5db537e1d048 src/main/python/apache/aurora/executor/aurora_executor.py 8985aeb60e2c890113ea7d25039cdab5b7c9a19e src/main/python/apache/aurora/executor/bin/BUILD 93517150db96600c92ea25c0de29ec20736d72f7 src/main/python/apache/aurora/executor/bin/gc_executor_main.py bc20dad1e5434a98fb51ab052efa8ced760e74e0 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py aacc19a3bd272b08dee8a77d7a64490d0fcad3fa src/main/python/apache/aurora/executor/common/BUILD 0a4d35afcbfca8730d8bb7373795f9b3b6a74fd7 src/main/python/apache/aurora/executor/common/health_checker.py 671f4970d4637b8cb68ec89e5dfe96a2f4cd077b src/main/python/apache/aurora/executor/common/kill_manager.py 6e482cfa7d2e081804fe6992527bae0ac5b65080 src/main/python/apache/aurora/executor/common/status_checker.py eda30e38d34adc5c78b828d0aea31f0721bf9b20 src/main/python/apache/aurora/executor/executor_base.py c632b77d972f76f3e260bb7413aa6100cda46607 src/main/python/apache/aurora/executor/gc_executor.py c45d5eb9a99e310e0716abd626ca34615f63b0ae src/main/python/apache/aurora/executor/thermos_task_runner.py 03a40e63573322f2d40e59a8ce84e9b9a1b90aa6 src/test/python/apache/aurora/executor/BUILD 3152ec0c0e7dfaec5d756542a89243756b27725d src/test/python/apache/aurora/executor/common/BUILD 6316c7fc724c8e2245f5ce1a93c4a2daecd9f26b src/test/python/apache/aurora/executor/common/test_executor_timeout.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py baeaba29e7679203d83f6ea81455973baf07f324 src/test/python/apache/aurora/executor/common/test_kill_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_status_checker.py 6ca4fd469bdc7cd21033ccdea460292463bc1cf8 src/test/python/apache/aurora/executor/test_executor_base.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 0b2278a94667ca4d43e8a84649ed5312acacf7bb src/test/python/apache/aurora/executor/test_status_manager.py PRE-CREATION src/test/python/apache/aurora/executor/test_thermos_executor.py 55747a19478572e39d1f00f799727d3c3db174c9 src/test/python/apache/aurora/executor/test_thermos_task_runner.py f7af70fda8fef91018ff9d10dc1db6d43fd16ac7 Diff: https://reviews.apache.org/r/25208/diff/ Testing --- $ ./gradlew test [tw-mbp13-jsmith aurora (yasumoto/mesos_0.20.0)]$ ./build-support/python/checkstyle-check ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager
Re: Review Request 25285: Upgrade to latest in jetty 7.x series.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/#review52203 --- Ship it! Ship It! - David McLaughlin On Sept. 3, 2014, 5:23 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/ --- (Updated Sept. 3, 2014, 5:23 a.m.) Review request for Aurora, David McLaughlin and Kevin Sweeney. Bugs: AURORA-679 https://issues.apache.org/jira/browse/AURORA-679 Repository: aurora Description --- Long story short: this change adds code, but enables us to remove code in a follow-up. This change brings us to Jetty 7, which is the newest jetty that still uses servlet-api 2.5. Jetty 8/9 will require a larger yak shave since it pulls a dependency thread all the way to guice (servlet-api - jersey-guice - guice-servlet - guice). This is also a small step towards insulating ourselves from transitive dependencies pulled in from twitter commons. [1] http://wiki.eclipse.org/Jetty/Starting/Porting_to_Jetty_7 Diffs - build.gradle 66c8205c8354048c398623edf7ba75e721376b01 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/RequestLoggerTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25285/diff/ Testing --- ./gradlew build -Pq Ran the scheduler in vagrant, verified index page works, request logging works, and all pages linked from the index page work. Thanks, Bill Farner
Re: Review Request 25206: Fix aurora run in end to end tests.
On Sept. 2, 2014, 6:47 p.m., Joe Smith wrote: src/test/sh/org/apache/aurora/e2e/test_run.sh, line 23 https://reviews.apache.org/r/25206/diff/1/?file=672554#file672554line23 aurora job run? Joe Smith wrote: Kevin and Toby, can you two take a look at this ASAP? I'd like to have the e2e tests fixed before I ship https://reviews.apache.org/r/25208/ Joshua Cohen wrote: It defaults to 'aurora run', for the aurora1 client, but we pass in 'aurora2 job run' from the test_end_to_end_v2.sh script. Are you saying to switch the default (or maybe I should just remove the default altogether)? I was saying switch the default so it's the new syntax - Joe --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52121 --- On Aug. 29, 2014, 4:17 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Aug. 29, 2014, 4:17 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25208: Increment Mesos version to 0.20.0
On Sept. 3, 2014, 10:42 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/executor/thermos_task_runner.py, line 23 https://reviews.apache.org/r/25208/diff/5/?file=674717#file674717line23 mesos_pb2 looks a bit cryptic here. How about the approach you sued elsewhere: from mesos.interface.mesos_pb2 import TaskState I'd like to keep it this way, as we don't want to clobber Thermos' thrift's TaskState[1] [1] https://github.com/apache/incubator-aurora/blob/master/src/main/thrift/org/apache/thermos/thermos_internal.thrift#L57 - Joe --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25208/#review52198 --- On Sept. 2, 2014, 9:06 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25208/ --- (Updated Sept. 2, 2014, 9:06 p.m.) Review request for Aurora, Kevin Sweeney, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-674 https://issues.apache.org/jira/browse/AURORA-674 Repository: aurora Description --- There's an API change when moving the python executor to 0.20.0, which should deserve extra scrutiny. Diffs - 3rdparty/python/BUILD edc446e6d76d53a4c10a703cf6e84e29b8d42366 Vagrantfile c8cb2d440330780c10eeff47d46cc1a3f1aa4910 build.gradle 66c8205c8354048c398623edf7ba75e721376b01 examples/vagrant/provision-dev-cluster.sh 2ce74f8a6dc663b62d254bc9d3a5c06cb412107f src/main/python/apache/aurora/executor/BUILD c1c50dbcdcaa56f8ff970ceac59d5db537e1d048 src/main/python/apache/aurora/executor/aurora_executor.py 8985aeb60e2c890113ea7d25039cdab5b7c9a19e src/main/python/apache/aurora/executor/bin/BUILD 93517150db96600c92ea25c0de29ec20736d72f7 src/main/python/apache/aurora/executor/bin/gc_executor_main.py bc20dad1e5434a98fb51ab052efa8ced760e74e0 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py aacc19a3bd272b08dee8a77d7a64490d0fcad3fa src/main/python/apache/aurora/executor/common/BUILD 0a4d35afcbfca8730d8bb7373795f9b3b6a74fd7 src/main/python/apache/aurora/executor/common/health_checker.py 671f4970d4637b8cb68ec89e5dfe96a2f4cd077b src/main/python/apache/aurora/executor/common/kill_manager.py 6e482cfa7d2e081804fe6992527bae0ac5b65080 src/main/python/apache/aurora/executor/common/status_checker.py eda30e38d34adc5c78b828d0aea31f0721bf9b20 src/main/python/apache/aurora/executor/executor_base.py c632b77d972f76f3e260bb7413aa6100cda46607 src/main/python/apache/aurora/executor/gc_executor.py c45d5eb9a99e310e0716abd626ca34615f63b0ae src/main/python/apache/aurora/executor/thermos_task_runner.py 03a40e63573322f2d40e59a8ce84e9b9a1b90aa6 src/test/python/apache/aurora/executor/BUILD 3152ec0c0e7dfaec5d756542a89243756b27725d src/test/python/apache/aurora/executor/common/BUILD 6316c7fc724c8e2245f5ce1a93c4a2daecd9f26b src/test/python/apache/aurora/executor/common/test_executor_timeout.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py baeaba29e7679203d83f6ea81455973baf07f324 src/test/python/apache/aurora/executor/common/test_kill_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_status_checker.py 6ca4fd469bdc7cd21033ccdea460292463bc1cf8 src/test/python/apache/aurora/executor/test_executor_base.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 0b2278a94667ca4d43e8a84649ed5312acacf7bb src/test/python/apache/aurora/executor/test_status_manager.py PRE-CREATION src/test/python/apache/aurora/executor/test_thermos_executor.py 55747a19478572e39d1f00f799727d3c3db174c9 src/test/python/apache/aurora/executor/test_thermos_task_runner.py f7af70fda8fef91018ff9d10dc1db6d43fd16ac7 Diff: https://reviews.apache.org/r/25208/diff/ Testing --- $ ./gradlew test [tw-mbp13-jsmith aurora (yasumoto/mesos_0.20.0)]$ ./build-support/python/checkstyle-check ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector .
Re: Review Request 25297: Make config-file an optional parameter for job restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/#review52208 --- Ship it! Ship It! - David McLaughlin On Sept. 3, 2014, 2:07 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/ --- (Updated Sept. 3, 2014, 2:07 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-673 https://issues.apache.org/jira/browse/aurora-673 Repository: aurora Description --- Make config-file an optional parameter for job restart. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 Diff: https://reviews.apache.org/r/25297/diff/ Testing --- - Ran all unit tests successfully. - Ran end-to-end tests successfully. - Thanks, Mark Chu-Carroll
Re: Review Request 25204: Adding get job update client APIs.
On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/__init__.py, line 176 https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line176 Nit - but why are you changing the parameter comment syntax? We don't use the double-dash anywhere else in the client. This is the style everything under client/api seems to follow. Just making it consistent. On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/__init__.py, line 232 https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line232 I think this would be clearer inlined. Right now, it's pretty much an alternate name for the JobUpdateQuery constructor, but with different parameter names. It makes the code harder to follow, not easier. The reason for this is mostly consistency (see build_query above) but I personally prefer this approach as it decouples query building from the thrift API call. On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/__init__.py, line 252 https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line252 If we're going to the trouble of abstracting away the actual scheduler interface, I think we shouldn't be returning the raw Response datatype. This should return the updates, not the structure wrapping the updates in two layers of indirection. These kinds of methods should check the result code, raise an exception if the API call failed, and then return a meaningful, simple result. (Same comment applies to get_job_update_details) Make it 3 for consistency :) Nothing in that file attempts to return a result. Event the updater tries to fake a Response object in order to comply with general api pattern. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52064 --- On Aug. 29, 2014, 10:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/ --- (Updated Aug. 29, 2014, 10:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-615 https://issues.apache.org/jira/browse/AURORA-615 Repository: aurora Description --- Adding get job update client APIs. Diffs - src/main/python/apache/aurora/client/api/__init__.py 90462bf5920786ea1316c75a99c8382cf8c803a1 src/test/python/apache/aurora/client/api/test_api.py b47b6db31842fffba797c7f616b5f4deb8d04a86 Diff: https://reviews.apache.org/r/25204/diff/ Testing --- ./pants src/test/python/apache/aurora/client/api:api -s Thanks, Maxim Khutornenko
Re: Review Request 25206: Fix aurora run in end to end tests.
At the moment, there's two end-to-end tests: test_end_to_end.sh, and
test_end_to_end_v2.sh. Since this is the v1 test, it should use v1.
-Mark
On Wed, Sep 3, 2014 at 2:05 PM, Joe Smith yasumo...@gmail.com wrote:
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25206/
On September 2nd, 2014, 6:47 p.m. PDT, *Joe Smith* wrote:
src/test/sh/org/apache/aurora/e2e/test_run.sh
https://reviews.apache.org/r/25206/diff/1/?file=672554#file672554line23
(Diff
revision 1)
23
aurora_command=${3:-aurora run}
aurora job run?
On September 2nd, 2014, 8:15 p.m. PDT, *Joe Smith* wrote:
Kevin and Toby, can you two take a look at this ASAP? I'd like to have the
e2e tests fixed before I ship https://reviews.apache.org/r/25208/
On September 3rd, 2014, 9:25 a.m. PDT, *Joshua Cohen* wrote:
It defaults to 'aurora run', for the aurora1 client, but we pass in 'aurora2
job run' from the test_end_to_end_v2.sh script. Are you saying to switch the
default (or maybe I should just remove the default altogether)?
I was saying switch the default so it's the new syntax
- Joe
On August 29th, 2014, 4:17 p.m. PDT, Joshua Cohen wrote:
Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias
Weingartner.
By Joshua Cohen.
*Updated Aug. 29, 2014, 4:17 p.m.*
*Bugs: * AURORA-676 https://issues.apache.org/jira/browse/AURORA-676
*Repository: * aurora
Description
Fix aurora run in end to end tests.
Testing
$ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
Unfortunately the v2 client end to end tests are currently broken (filed
https://issues.apache.org/jira/browse/AURORA-677), but I verified that the
test for aurora task run works in that script as well.
Diffs
- examples/vagrant/upstart/mesos-slave.conf
(e53d545f3a66383c12ed1e7de6b1eef2dbad1541)
- src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
(fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f)
- src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh
(14a0b62c1b06179211d79f35a9c6df162c9a5999)
- src/test/sh/org/apache/aurora/e2e/test_run.sh (PRE-CREATION)
View Diff https://reviews.apache.org/r/25206/diff/
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52210 --- Ship it! examples/vagrant/upstart/mesos-slave.conf https://reviews.apache.org/r/25206/#comment90955 Can you add a comment explaining why this is necessary (preferably with a link to a bug where we will fix this in the client - the clusters.json default should match the mesos default). - Kevin Sweeney On Aug. 29, 2014, 4:17 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Aug. 29, 2014, 4:17 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 6:13 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Changes --- Switch default in test_run.sh to use aurora2 command. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs (updated) - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Review Request 25309: Fix output formatting error in job status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Re: Review Request 25309: Fix output formatting error in job status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/#review52212 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 3, 2014, 6:17 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- (Updated Sept. 3, 2014, 6:17 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Re: Review Request 25204: Adding get job update client APIs.
On Sept. 2, 2014, 3:35 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/__init__.py, line 176 https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line176 Nit - but why are you changing the parameter comment syntax? We don't use the double-dash anywhere else in the client. Maxim Khutornenko wrote: This is the style everything under client/api seems to follow. Just making it consistent. Ick. So client/api doesn't follow the same pattern as a lot of other client code. Something for me to fix at some point. On Sept. 2, 2014, 3:35 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/__init__.py, line 232 https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line232 I think this would be clearer inlined. Right now, it's pretty much an alternate name for the JobUpdateQuery constructor, but with different parameter names. It makes the code harder to follow, not easier. Maxim Khutornenko wrote: The reason for this is mostly consistency (see build_query above) but I personally prefer this approach as it decouples query building from the thrift API call. In this case, I disagree pretty strongly - but if you must have the separate method, at least make the argument names match. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52064 --- On Aug. 29, 2014, 6:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/ --- (Updated Aug. 29, 2014, 6:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-615 https://issues.apache.org/jira/browse/AURORA-615 Repository: aurora Description --- Adding get job update client APIs. Diffs - src/main/python/apache/aurora/client/api/__init__.py 90462bf5920786ea1316c75a99c8382cf8c803a1 src/test/python/apache/aurora/client/api/test_api.py b47b6db31842fffba797c7f616b5f4deb8d04a86 Diff: https://reviews.apache.org/r/25204/diff/ Testing --- ./pants src/test/python/apache/aurora/client/api:api -s Thanks, Maxim Khutornenko
Re: Review Request 25204: Adding get job update client APIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52214 --- Ship it! Ship It! - Mark Chu-Carroll On Aug. 29, 2014, 6:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/ --- (Updated Aug. 29, 2014, 6:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-615 https://issues.apache.org/jira/browse/AURORA-615 Repository: aurora Description --- Adding get job update client APIs. Diffs - src/main/python/apache/aurora/client/api/__init__.py 90462bf5920786ea1316c75a99c8382cf8c803a1 src/test/python/apache/aurora/client/api/test_api.py b47b6db31842fffba797c7f616b5f4deb8d04a86 Diff: https://reviews.apache.org/r/25204/diff/ Testing --- ./pants src/test/python/apache/aurora/client/api:api -s Thanks, Maxim Khutornenko
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 6:33 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Changes --- Added comment explaining --work_dir param when starting up the mesos slaves. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs (updated) - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25206: Fix aurora run in end to end tests.
On Sept. 3, 2014, 1:47 a.m., Joe Smith wrote: src/test/sh/org/apache/aurora/e2e/test_run.sh, line 23 https://reviews.apache.org/r/25206/diff/1/?file=672554#file672554line23 aurora job run? Joe Smith wrote: Kevin and Toby, can you two take a look at this ASAP? I'd like to have the e2e tests fixed before I ship https://reviews.apache.org/r/25208/ Joshua Cohen wrote: It defaults to 'aurora run', for the aurora1 client, but we pass in 'aurora2 job run' from the test_end_to_end_v2.sh script. Are you saying to switch the default (or maybe I should just remove the default altogether)? Joe Smith wrote: I was saying switch the default so it's the new syntax Updated. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52121 --- On Sept. 3, 2014, 6:33 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 6:33 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25206: Fix aurora run in end to end tests.
On Sept. 3, 2014, 6:10 p.m., Kevin Sweeney wrote: examples/vagrant/upstart/mesos-slave.conf, line 28 https://reviews.apache.org/r/25206/diff/1/?file=672551#file672551line28 Can you add a comment explaining why this is necessary (preferably with a link to a bug where we will fix this in the client - the clusters.json default should match the mesos default). Added. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52210 --- On Sept. 3, 2014, 6:33 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 6:33 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25285: Upgrade to latest in jetty 7.x series.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/#review52221 --- Ship it! This patch actually pulls in the bit of twitter.common.http that I would need to change to get ShiroWebModule wired in, so big +1 from me - Kevin Sweeney On Sept. 2, 2014, 10:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/ --- (Updated Sept. 2, 2014, 10:23 p.m.) Review request for Aurora, David McLaughlin and Kevin Sweeney. Bugs: AURORA-679 https://issues.apache.org/jira/browse/AURORA-679 Repository: aurora Description --- Long story short: this change adds code, but enables us to remove code in a follow-up. This change brings us to Jetty 7, which is the newest jetty that still uses servlet-api 2.5. Jetty 8/9 will require a larger yak shave since it pulls a dependency thread all the way to guice (servlet-api - jersey-guice - guice-servlet - guice). This is also a small step towards insulating ourselves from transitive dependencies pulled in from twitter commons. [1] http://wiki.eclipse.org/Jetty/Starting/Porting_to_Jetty_7 Diffs - build.gradle 66c8205c8354048c398623edf7ba75e721376b01 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/RequestLoggerTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25285/diff/ Testing --- ./gradlew build -Pq Ran the scheduler in vagrant, verified index page works, request logging works, and all pages linked from the index page work. Thanks, Bill Farner
Re: Review Request 25300: Add a state machine to react to job update status changes.
On Sept. 3, 2014, 5:33 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java, lines 74-76 https://reviews.apache.org/r/25300/diff/1/?file=675222#file675222line74 This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED - ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift APIs (i.e. pause/resume/abort). Would it make sense to alter ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if from != to? I didn't see much value in supporting no-op transitions. The caller can suppress that easily enough, so i chose to be more restrictive here. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/#review52195 --- On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/ --- (Updated Sept. 3, 2014, 4:02 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This will be used by the job updater to gate insertion of JobUpdateEvents. I chose not to use StateMachine here as it actually added complexity - in this case we don't benefit from transition callbacks, and the result is based only on the target state (rather than the transition). Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner
Re: Review Request 25175: Fix possible deadlock in TaskRunner.collect_updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/#review52223 --- src/main/python/apache/thermos/core/runner.py https://reviews.apache.org/r/25175/#comment90965 This changes the return value from 0 to None upon timeout. Perhaps change 'break' to 'return 0'? - David Robinson On Aug. 28, 2014, 11:34 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/ --- (Updated Aug. 28, 2014, 11:34 p.m.) Review request for Aurora. Bugs: AURORA-669 https://issues.apache.org/jira/browse/AURORA-669 Repository: aurora Description --- Fix possible deadlock in TaskRunner.collect_updates. Diffs - src/main/python/apache/thermos/core/runner.py ec4cdb788a1e24a590b262ba8dbd044cf31fc198 Diff: https://reviews.apache.org/r/25175/diff/ Testing --- Regression test y/n ? Thanks, Brian Wickman
Re: Review Request 25300: Add a state machine to react to job update status changes.
On Sept. 3, 2014, 5:33 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java, lines 74-76 https://reviews.apache.org/r/25300/diff/1/?file=675222#file675222line74 This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED - ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift APIs (i.e. pause/resume/abort). Would it make sense to alter ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if from != to? Bill Farner wrote: I didn't see much value in supporting no-op transitions. The caller can suppress that easily enough, so i chose to be more restrictive here. I guess I don't see how the caller would be able to set apart illegal state transition (valid error) from the same state one. Are you suggesting querying the state before trying to call into the controller? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/#review52195 --- On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/ --- (Updated Sept. 3, 2014, 4:02 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This will be used by the job updater to gate insertion of JobUpdateEvents. I chose not to use StateMachine here as it actually added complexity - in this case we don't benefit from transition callbacks, and the result is based only on the target state (rather than the transition). Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner
Re: Review Request 25300: Add a state machine to react to job update status changes.
On Sept. 3, 2014, 5:33 p.m., Maxim Khutornenko wrote:
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java,
lines 74-76
https://reviews.apache.org/r/25300/diff/1/?file=675222#file675222line74
This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED
- ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift
APIs (i.e. pause/resume/abort). Would it make sense to alter
ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if
from != to?
Bill Farner wrote:
I didn't see much value in supporting no-op transitions. The caller can
suppress that easily enough, so i chose to be more restrictive here.
Maxim Khutornenko wrote:
I guess I don't see how the caller would be able to set apart illegal
state transition (valid error) from the same state one. Are you suggesting
querying the state before trying to call into the controller?
I'm suggesting that the caller avoid invoking transition(a, b) where a==b. If
you'd prefer, i can allow that and return a NO_OP action, but all that really
does is move code around. I don't have a strong opinion, neither seems more
attractive:
```
if (current != next) {
switch (transition(current, next)) {
case STOP_WATCHING:
...
}
}
```
```
switch (transition(current, next)) {
case NO_OP:
break;
case STOP_WATCHING:
...
}
```
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25300/#review52195
---
On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25300/
---
(Updated Sept. 3, 2014, 4:02 p.m.)
Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
---
This will be used by the job updater to gate insertion of JobUpdateEvents.
I chose not to use StateMachine here as it actually added complexity - in
this case we don't benefit from transition callbacks, and the result is based
only on the target state (rather than the transition).
Diffs
-
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java
PRE-CREATION
Diff: https://reviews.apache.org/r/25300/diff/
Testing
---
`./gradlew build -Pq`
Thanks,
Bill Farner
Re: Review Request 25300: Add a state machine to react to job update status changes.
On Sept. 3, 2014, 5:33 p.m., Maxim Khutornenko wrote:
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java,
lines 74-76
https://reviews.apache.org/r/25300/diff/1/?file=675222#file675222line74
This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED
- ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift
APIs (i.e. pause/resume/abort). Would it make sense to alter
ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if
from != to?
Bill Farner wrote:
I didn't see much value in supporting no-op transitions. The caller can
suppress that easily enough, so i chose to be more restrictive here.
Maxim Khutornenko wrote:
I guess I don't see how the caller would be able to set apart illegal
state transition (valid error) from the same state one. Are you suggesting
querying the state before trying to call into the controller?
Bill Farner wrote:
I'm suggesting that the caller avoid invoking transition(a, b) where
a==b. If you'd prefer, i can allow that and return a NO_OP action, but all
that really does is move code around. I don't have a strong opinion, neither
seems more attractive:
```
if (current != next) {
switch (transition(current, next)) {
case STOP_WATCHING:
...
}
}
```
```
switch (transition(current, next)) {
case NO_OP:
break;
case STOP_WATCHING:
...
}
```
Just realized this may be a case of different definitions of _caller_. My
_caller_ is the next level up in the call stack, sounds yours is an HTTP API
consumer.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25300/#review52195
---
On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25300/
---
(Updated Sept. 3, 2014, 4:02 p.m.)
Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
---
This will be used by the job updater to gate insertion of JobUpdateEvents.
I chose not to use StateMachine here as it actually added complexity - in
this case we don't benefit from transition callbacks, and the result is based
only on the target state (rather than the transition).
Diffs
-
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java
PRE-CREATION
Diff: https://reviews.apache.org/r/25300/diff/
Testing
---
`./gradlew build -Pq`
Thanks,
Bill Farner
Re: Review Request 25300: Add a state machine to react to job update status changes.
On Sept. 3, 2014, 5:33 p.m., Maxim Khutornenko wrote:
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java,
lines 74-76
https://reviews.apache.org/r/25300/diff/1/?file=675222#file675222line74
This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED
- ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift
APIs (i.e. pause/resume/abort). Would it make sense to alter
ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if
from != to?
Bill Farner wrote:
I didn't see much value in supporting no-op transitions. The caller can
suppress that easily enough, so i chose to be more restrictive here.
Maxim Khutornenko wrote:
I guess I don't see how the caller would be able to set apart illegal
state transition (valid error) from the same state one. Are you suggesting
querying the state before trying to call into the controller?
Bill Farner wrote:
I'm suggesting that the caller avoid invoking transition(a, b) where
a==b. If you'd prefer, i can allow that and return a NO_OP action, but all
that really does is move code around. I don't have a strong opinion, neither
seems more attractive:
```
if (current != next) {
switch (transition(current, next)) {
case STOP_WATCHING:
...
}
}
```
```
switch (transition(current, next)) {
case NO_OP:
break;
case STOP_WATCHING:
...
}
```
Bill Farner wrote:
Just realized this may be a case of different definitions of _caller_.
My _caller_ is the next level up in the call stack, sounds yours is an HTTP
API consumer.
Yes, the missing context really helps here. I was assuming this would be
directly exposed to the scheduler API. The first example makes sense (i.e.
no-op seems redundant). Thanks for explaining.
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25300/#review52195
---
On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25300/
---
(Updated Sept. 3, 2014, 4:02 p.m.)
Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
---
This will be used by the job updater to gate insertion of JobUpdateEvents.
I chose not to use StateMachine here as it actually added complexity - in
this case we don't benefit from transition callbacks, and the result is based
only on the target state (rather than the transition).
Diffs
-
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java
PRE-CREATION
Diff: https://reviews.apache.org/r/25300/diff/
Testing
---
`./gradlew build -Pq`
Thanks,
Bill Farner
Re: Review Request 25300: Add a state machine to react to job update status changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/#review52231 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25300/ --- (Updated Sept. 3, 2014, 4:02 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This will be used by the job updater to gate insertion of JobUpdateEvents. I chose not to use StateMachine here as it actually added complexity - in this case we don't benefit from transition callbacks, and the result is based only on the target state (rather than the transition). Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner
Review Request 25311: Checking for cron jobs in startJobUpdate()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25311/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Cron jobs should not be allowed in startJobUpdate(). Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7a4640d8ca8a8a080368e27af5a1f6f83ea2f95d src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f5682801902589552907415b4563af5b069af929 Diff: https://reviews.apache.org/r/25311/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25311: Checking for cron jobs in startJobUpdate()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25311/#review52232 --- Ship it! src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java https://reviews.apache.org/r/25311/#comment90973 Cron jobs may only be updated by calling replaceCronTemplate. - Bill Farner On Sept. 3, 2014, 7:41 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25311/ --- (Updated Sept. 3, 2014, 7:41 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Cron jobs should not be allowed in startJobUpdate(). Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7a4640d8ca8a8a080368e27af5a1f6f83ea2f95d src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f5682801902589552907415b4563af5b069af929 Diff: https://reviews.apache.org/r/25311/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25255: Implement server-driven update commands.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/#review52220 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/25255/#comment90978 Why not reuse the existing update verb? Having supdate just to avoid naming collision with the existing verb that will go away soon does not seem well justified. src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/25255/#comment90962 How about a BROWSER_OPTION for all update commands (start/pause/resume/abort)? src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/25255/#comment90972 Just realized we are missing cron-check on the server side (https://reviews.apache.org/r/25311/). You might want to do a similar check here to short circuit server call and provide a custom error message hinting users to use aurora cron commands. src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/25255/#comment90974 Is this a result of sharing the verb definition with start_update? Any chance to avoid sharing the option set here? - Maxim Khutornenko On Sept. 2, 2014, 4:36 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/ --- (Updated Sept. 2, 2014, 4:36 p.m.) Review request for Aurora. Repository: aurora Description --- This change contains the basic commands needed to work with the scheduler-driven updater. (It does not yet cover querying for the status of the update; that will come in a subsequent change.) Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 Diff: https://reviews.apache.org/r/25255/diff/ Testing --- New suite of tests for the new command; all unit tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52235 --- Ship it! Ship It! - David Robinson On Sept. 3, 2014, 6:33 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 6:33 p.m.) Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Tobias Weingartner. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25311: Checking for cron jobs in startJobUpdate()
On Sept. 3, 2014, 7:49 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1343 https://reviews.apache.org/r/25311/diff/1/?file=675806#file675806line1343 Cron jobs may only be updated by calling replaceCronTemplate. Sure, why not. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25311/#review52232 --- On Sept. 3, 2014, 7:41 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25311/ --- (Updated Sept. 3, 2014, 7:41 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Cron jobs should not be allowed in startJobUpdate(). Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7a4640d8ca8a8a080368e27af5a1f6f83ea2f95d src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f5682801902589552907415b4563af5b069af929 Diff: https://reviews.apache.org/r/25311/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25311: Checking for cron jobs in startJobUpdate()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25311/ --- (Updated Sept. 3, 2014, 8:52 p.m.) Review request for Aurora and Bill Farner. Changes --- CR comments. Repository: aurora Description --- Cron jobs should not be allowed in startJobUpdate(). Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7a4640d8ca8a8a080368e27af5a1f6f83ea2f95d src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f5682801902589552907415b4563af5b069af929 Diff: https://reviews.apache.org/r/25311/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 8:55 p.m.) Review request for Aurora, David Robinson, Kevin Sweeney, and Mark Chu-Carroll. Changes --- -tweingartner, +drobinson from reviewers. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25255: Implement server-driven update commands.
On Sept. 2, 2014, 1:32 p.m., Joshua Cohen wrote: Is the command name supdate up for debate? I'm not in love with it ;). It's a temporary thing. (Came up with it during a discussion with Bill.) If we start adding this stuff to the existing update command, then existing users are going to see things like pause, and resume that don't make any sense, and that are only related to a component/service that they can't use. We don't want to screw up or confuse users of the existing command during the development of the update service - so for that transition, we're keeping update/supdate separate. I'm not in love with supdate as the name, but I didn't want to waste time coming up with something better when it's just temporary anyway. On Sept. 2, 2014, 1:32 p.m., Joshua Cohen wrote: src/main/python/apache/aurora/client/cli/jobs.py, lines 664-665 https://reviews.apache.org/r/25255/diff/1/?file=673959#file673959line664 This doesn't read very well to me. Maybe something like: Update action to start an update or pause, resume or abort an in-progress update. I think that wording is worse... The help should suggest what the value of the parameter is. Update action to start an update or pause... blurs that. The argument specifies the action that the user wants, which can be update, pause, resume, or abort. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/#review52043 --- On Sept. 2, 2014, 12:36 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/ --- (Updated Sept. 2, 2014, 12:36 p.m.) Review request for Aurora. Repository: aurora Description --- This change contains the basic commands needed to work with the scheduler-driven updater. (It does not yet cover querying for the status of the update; that will come in a subsequent change.) Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 Diff: https://reviews.apache.org/r/25255/diff/ Testing --- New suite of tests for the new command; all unit tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 24815: Refactoring SchedulerCore final part.
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java, line 120 https://reviews.apache.org/r/24815/diff/1/?file=662756#file662756line120 At first glance, this is odd since ITaskConfig contains the components of a JobKey. Is that argument necessary? I tried and really hated that approach. Adding a bit of redunancy helped avoiding clumsy job key extraction and edge case error conditions (e.g. empty collection, ITaskConfigs from different jobs and etc.) On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, line 35 https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35 Is this interface useful? The implementation logic is trivial, and it's only used from SchedulerThriftInterface. I suggest putting it there. Right now that is correct. However, I'd expect JobUpdateController to accept it in order to check quota any time an instance about to be added. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/#review52089 --- On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/ --- (Updated Aug. 18, 2014, 8:04 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Moving the last bits of functionality out of SchedulerCore. Also, preparing the ground for task validation logic reuse in updater code. Diffs - src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java e060e5ec88a0ab311415eaa638cf693c99c40049 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 62202810fd5829545fc77b91a20d1bb433a4916a src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java c636fd7fde8b592b167da8e5e9651ac772bc23de src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f src/main/java/org/apache/aurora/scheduler/state/StateManager.java 6e062b39175255264020bbb1cbd485de9a114a20 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6ad104b050aabecad1dadc975c27d9d3603659bf src/main/java/org/apache/aurora/scheduler/state/StateModule.java cc1eee4e19c092c0d401558ac01b54627f2d1290 src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7ef28858ad290c74248b89c49d2a684eb1c7127e src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java c7ae7db16e82c722fae1bccb9178f5ec203e91e5 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 62dce07b42af02d25b788e763e4e2a026dd2d483 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java fa611a913bad40a8c0515c578b394c460340e574 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 1678411ad5c25e74f8b6d004bf491ea26ca0 src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java 35bed104d838596abcbb5abd5cad29592b384dfa src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 649afa24b2cfc9a1d67d350473e439d209bd720c src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 43265fdab1ae900fb828374f6c69e562def2d682 Diff: https://reviews.apache.org/r/24815/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/#review52245 --- I ran this before committing and it still fails (aurora run hangs). Looks like flags need to be pushed to the server-side ssh - Kevin Sweeney On Sept. 3, 2014, 1:55 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 1:55 p.m.) Review request for Aurora, David Robinson, Kevin Sweeney, and Mark Chu-Carroll. Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs - examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
On Sept. 2, 2014, 4:43 p.m., Bill Farner wrote: I'm concerned that the problem we're solving is underspecified. An immediate issue i have with this patch is that it introduces a memory leak (in practice, this is hidden with default settings due to failover failover hides this). I'll reply on the ticket to try to reach consensus on what the problem is and what it means to fix it. I don't think we are trying to fix a Mesos problem here. Regardless of the underlying Mesos resolution (MESOS-1646), I do think Aurora should be a good Mesos citizen here and avoid abnormal offer activity spikes driven by failovers. In order for that memory leak to expose itself in any reasonable manner, there must be an intensive and constant churn of hosts in the cluster. I don't think it's a likely scenario but if you are concerned it should be easy to converge on a hybrid solution with expiring cache where the EXECUTOR_GC_RESTART_INTERVAL kicks in any time an item is not found in the cache. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52037 --- On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25204: Adding get job update client APIs.
On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/__init__.py, line 232 https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line232 I think this would be clearer inlined. Right now, it's pretty much an alternate name for the JobUpdateQuery constructor, but with different parameter names. It makes the code harder to follow, not easier. Maxim Khutornenko wrote: The reason for this is mostly consistency (see build_query above) but I personally prefer this approach as it decouples query building from the thrift API call. Mark Chu-Carroll wrote: In this case, I disagree pretty strongly - but if you must have the separate method, at least make the argument names match. Feels like your disagreement is stronger than my preference in this case :). Changed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52064 --- On Aug. 29, 2014, 10:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/ --- (Updated Aug. 29, 2014, 10:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-615 https://issues.apache.org/jira/browse/AURORA-615 Repository: aurora Description --- Adding get job update client APIs. Diffs - src/main/python/apache/aurora/client/api/__init__.py 90462bf5920786ea1316c75a99c8382cf8c803a1 src/test/python/apache/aurora/client/api/test_api.py b47b6db31842fffba797c7f616b5f4deb8d04a86 Diff: https://reviews.apache.org/r/25204/diff/ Testing --- ./pants src/test/python/apache/aurora/client/api:api -s Thanks, Maxim Khutornenko
Re: Review Request 25206: Fix aurora run in end to end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25206/ --- (Updated Sept. 3, 2014, 10:40 p.m.) Review request for Aurora, David Robinson, Kevin Sweeney, and Mark Chu-Carroll. Changes --- Allow local ssh w/out strict host checking Bugs: AURORA-676 https://issues.apache.org/jira/browse/AURORA-676 Repository: aurora Description --- Fix aurora run in end to end tests. Diffs (updated) - examples/vagrant/provision-dev-cluster.sh 2ce74f8a6dc663b62d254bc9d3a5c06cb412107f examples/vagrant/upstart/mesos-slave.conf e53d545f3a66383c12ed1e7de6b1eef2dbad1541 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh fa5568e5d34e1a8b2db78b4c9d009ed3d47ed04f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 14a0b62c1b06179211d79f35a9c6df162c9a5999 src/test/sh/org/apache/aurora/e2e/test_run.sh PRE-CREATION Diff: https://reviews.apache.org/r/25206/diff/ Testing --- $ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Unfortunately the v2 client end to end tests are currently broken (filed https://issues.apache.org/jira/browse/AURORA-677), but I verified that the test for aurora task run works in that script as well. Thanks, Joshua Cohen
Re: Review Request 25309: Fix output formatting error in job status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/#review52257 --- Ship it! Ship It! - David McLaughlin On Sept. 3, 2014, 6:17 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- (Updated Sept. 3, 2014, 6:17 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Review Request 25319: Tweak contributors doc to add details on finding newbie issues.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25319/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Tweak contributors doc to add details on finding newbie issues. I thought the Finding an Issue section made sense right at the top, let me know if that seems ok. Also, hopefully the tone is acceptable :). Diffs - docs/contributing.md 44512c273e5f24c2b9352b3a817c2a1bb5996ce4 Diff: https://reviews.apache.org/r/25319/diff/ Testing --- Thanks, Joshua Cohen
