Re: Review Request 29942: Updates so client will run in PyCharm.
On Jan. 15, 2015, 11:59 p.m., Aurora ReviewBot wrote: This patch does not apply cleanly on master (b75ed0f), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry I don't know what your deal is ReviewBot, it applied cleanly for me! - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29942/#review68365 --- On Jan. 15, 2015, 11:58 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29942/ --- (Updated Jan. 15, 2015, 11:58 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Updates so client will run in PyCharm. Diffs - docs/developing-aurora-client.md a7253d2beba61f7aca94bab944f544b969f507fd docs/images/debug-client-test.png PRE-CREATION docs/images/debugging-client-test.png PRE-CREATION examples/vagrant/clusters.json PRE-CREATION examples/vagrant/provision-dev-cluster.sh 7af4b52a6876268a97630279221bb98d9b04efad src/main/python/apache/aurora/client/cli/__init__.py a59594855bb82c1b4d1d28dfa2e1ac2d66aeb569 Diff: https://reviews.apache.org/r/29942/diff/ Testing --- Ran/debug client in PyCharm. Previewed markdown doc changes here: https://github.com/jcohen/incubator-aurora/blob/jcohen/client/run-in-pycharm/docs/developing-aurora-client.md Thanks, Joshua Cohen
Re: Review Request 29942: Updates so client will run in PyCharm.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29942/ --- (Updated Jan. 15, 2015, 11:58 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- rebase Repository: aurora Description --- Updates so client will run in PyCharm. Diffs (updated) - docs/developing-aurora-client.md a7253d2beba61f7aca94bab944f544b969f507fd docs/images/debug-client-test.png PRE-CREATION docs/images/debugging-client-test.png PRE-CREATION examples/vagrant/clusters.json PRE-CREATION examples/vagrant/provision-dev-cluster.sh 7af4b52a6876268a97630279221bb98d9b04efad src/main/python/apache/aurora/client/cli/__init__.py a59594855bb82c1b4d1d28dfa2e1ac2d66aeb569 Diff: https://reviews.apache.org/r/29942/diff/ Testing --- Ran/debug client in PyCharm. Previewed markdown doc changes here: https://github.com/jcohen/incubator-aurora/blob/jcohen/client/run-in-pycharm/docs/developing-aurora-client.md Thanks, Joshua Cohen
Re: Review Request 29984: Minor clean up to mock usage in resource manager integration tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/#review68754 --- @ReviewBoy retry - Joshua Cohen On Jan. 16, 2015, 9:45 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/ --- (Updated Jan. 16, 2015, 9:45 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Minor clean up to mock usage in resource manager integration tests. We didn't need to wrap the mock disk collector in a lambda, just rather than asserting on the mock class, we instead needed to assert on its return value (the instance). Diffs - src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py bb3045700c2870329e175db44873dcb990226c7a Diff: https://reviews.apache.org/r/29984/diff/ Testing --- ./pants build src/test/python/apache/aurora/executor/common:resource_manager_integration Thanks, Joshua Cohen
Re: Review Request 29984: Minor clean up to mock usage in resource manager integration tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/#review68828 --- *ping* - Joshua Cohen On Jan. 16, 2015, 9:45 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/ --- (Updated Jan. 16, 2015, 9:45 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Minor clean up to mock usage in resource manager integration tests. We didn't need to wrap the mock disk collector in a lambda, just rather than asserting on the mock class, we instead needed to assert on its return value (the instance). Diffs - src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py bb3045700c2870329e175db44873dcb990226c7a Diff: https://reviews.apache.org/r/29984/diff/ Testing --- ./pants build src/test/python/apache/aurora/executor/common:resource_manager_integration Thanks, Joshua Cohen
Re: Review Request 29984: Minor clean up to mock usage in resource manager integration tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/#review68829 --- @ReviewBot retry - Joshua Cohen On Jan. 16, 2015, 9:45 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/ --- (Updated Jan. 16, 2015, 9:45 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Minor clean up to mock usage in resource manager integration tests. We didn't need to wrap the mock disk collector in a lambda, just rather than asserting on the mock class, we instead needed to assert on its return value (the instance). Diffs - src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py bb3045700c2870329e175db44873dcb990226c7a Diff: https://reviews.apache.org/r/29984/diff/ Testing --- ./pants build src/test/python/apache/aurora/executor/common:resource_manager_integration Thanks, Joshua Cohen
Review Request 29942: Updates so client will run in PyCharm.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29942/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Updates so client will run in PyCharm. Diffs - docs/developing-aurora-client.md a7253d2beba61f7aca94bab944f544b969f507fd docs/images/debug-client-test.png PRE-CREATION docs/images/debugging-client-test.png PRE-CREATION examples/vagrant/clusters.json PRE-CREATION examples/vagrant/provision-dev-cluster.sh 7af4b52a6876268a97630279221bb98d9b04efad src/main/python/apache/aurora/client/cli/__init__.py a59594855bb82c1b4d1d28dfa2e1ac2d66aeb569 Diff: https://reviews.apache.org/r/29942/diff/ Testing --- Ran/debug client in PyCharm. Previewed markdown doc changes here: https://github.com/jcohen/incubator-aurora/blob/jcohen/client/run-in-pycharm/docs/developing-aurora-client.md Thanks, Joshua Cohen
Review Request 29971: Fix path to stylesheet in slaves and utilization templates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29971/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1019 https://issues.apache.org/jira/browse/AURORA-1019 Repository: aurora Description --- Fix path to stylesheet in slaves and utilization templates. Diffs - src/main/resources/org/apache/aurora/scheduler/http/slaves.st 05541f8f1deabc4b60001ac85b8fffa04b03ebf2 src/main/resources/org/apache/aurora/scheduler/http/utilization.st 73f86d51e9bb0179b774554718ad49ce00ba5487 Diff: https://reviews.apache.org/r/29971/diff/ Testing --- Verified styles were present when hitting those endpoints in vagrant. Thanks, Joshua Cohen
Re: Review Request 29971: Fix path to stylesheet in slaves and utilization templates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29971/#review68477 --- @ReviewBot retry - Joshua Cohen On Jan. 16, 2015, 7:08 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29971/ --- (Updated Jan. 16, 2015, 7:08 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1019 https://issues.apache.org/jira/browse/AURORA-1019 Repository: aurora Description --- Fix path to stylesheet in slaves and utilization templates. Diffs - src/main/resources/org/apache/aurora/scheduler/http/slaves.st 05541f8f1deabc4b60001ac85b8fffa04b03ebf2 src/main/resources/org/apache/aurora/scheduler/http/utilization.st 73f86d51e9bb0179b774554718ad49ce00ba5487 Diff: https://reviews.apache.org/r/29971/diff/ Testing --- Verified styles were present when hitting those endpoints in vagrant. Thanks, Joshua Cohen
Review Request 29984: Minor clean up to mock usage in resource manager integration tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29984/ --- Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Minor clean up to mock usage in resource manager integration tests. We didn't need to wrap the mock disk collector in a lambda, just rather than asserting on the mock class, we instead needed to assert on its return value (the instance). Diffs - src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py bb3045700c2870329e175db44873dcb990226c7a Diff: https://reviews.apache.org/r/29984/diff/ Testing --- ./pants build src/test/python/apache/aurora/executor/common:resource_manager_integration Thanks, Joshua Cohen
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
On Jan. 22, 2015, 9:30 p.m., Brian Wickman wrote: I think we should leave the yaml code but drop the PyYAML dependency from the client requirements. (And inject it into the test -- possibly one test with and one test without to make sure the try/except also functions correctly.) I feel that .yml is actually a much more natural way of defining the cluster list than json, because YAML supports basic templating in order to reduce redundancy, similar to pystachio. The way the code is currently structured, YAML will still work fine if it's available in the environmenet of your Aurora client, and gracefully fall back if not. Thoughts? Zameer Manji wrote: If we are going to drop the PyYAML dependency then I think we should drop all of the YAML related code. I agree that YAML is a much nicer format because of the templating functionality it has but as it stands I think the cost of supporting YAML (native code, extra compelxity in loading) is greater than the benefits of a simpler config file. I'm not sure the added complexity is worth the minor benefits provided by YAML. Is cluster configuration really so complex that it requires templating (and if so, could we rely on cluster administrators to work out their own mechanism for templatizing config, puppet/chef/etc.) - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69269 --- On Jan. 22, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 9:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72014 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java https://reviews.apache.org/r/30225/#comment117942 can drop the else? - Joshua Cohen On Feb. 11, 2015, 7:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72028 --- Ship it! Ship It! - Joshua Cohen On Feb. 11, 2015, 10:01 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 10:01 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72065 --- Ship it! pending clean test run from review bot. - Joshua Cohen On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 31137: Update aurora to commons 0.3.3, unflake tests using ThreadedClock
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31137/#review72810 --- Ship it! Ship It! - Joshua Cohen On Feb. 17, 2015, 9:59 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31137/ --- (Updated Feb. 17, 2015, 9:59 p.m.) Review request for Aurora, Joshua Cohen and Joe Smith. Repository: aurora Description --- SSIA. One seemingly incorrect test was removed. It was unclear to me what behavior it was testing exactly, and it did not consistently pass. Diffs - 3rdparty/python/requirements.txt 9f7bea9c0dce620c436332e1f4f4c9e3df7f66f5 src/main/python/apache/aurora/executor/gc_executor.py 43b415bc6c5177be24ec036cc32ae7cbd87fc70f src/test/python/apache/aurora/executor/common/test_health_checker.py def249c2509a28f7145380f250f79202b653dc83 src/test/python/apache/aurora/executor/test_gc_executor.py b1bbc89a822302d8ea12324eb767631326639ebb Diff: https://reviews.apache.org/r/31137/diff/ Testing --- ./pants test.pytest --no-fast src/test/python:: Thanks, Brian Wickman
Review Request 31138: Add ability to pass configurable options to pytest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31138/ --- Review request for Aurora, Bill Farner and Brian Wickman. Repository: aurora Description --- Add ability to pass configurable options to pytest. This will let us debug what test is causing review bot to hang (c.f. https://issues.apache.org/jira/browse/AURORA-1130) Diffs - build-support/jenkins/build.sh 6f9f017f00b49174dbd5b3f70a4923d89a5f51a1 Diff: https://reviews.apache.org/r/31138/diff/ Testing --- Ran build.sh before/after exporting PANTS_PYTEST_OPTIONS='-v' Thanks, Joshua Cohen
Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review72858 --- Ship it! Ship It! - Joshua Cohen On Feb. 18, 2015, 1 a.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 18, 2015, 1 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Review Request 30950: Add the option to make a non-hooked API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30950/ --- Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1120 https://issues.apache.org/jira/browse/AURORA-1120 Repository: aurora Description --- Add the option to make a non-hooked API. Diffs - src/main/python/apache/aurora/client/cli/context.py f8b289938201da1edd7d6dde2c65124b80b58adb src/main/python/apache/aurora/client/factory.py 85a1398106a575c1b7759b904b32db6b07da7c1b src/test/python/apache/aurora/client/BUILD c55adfe9825b77f418e41fa9a4ba43926bd991ed src/test/python/apache/aurora/client/cli/BUILD 7319962b285b449b275196eb0c4033e963579d8e src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 12360c64a6c6f46d5b96f604c002b59d0f1a9b0e src/test/python/apache/aurora/client/test_factory.py PRE-CREATION src/test/python/apache/aurora/client/util.py PRE-CREATION Diff: https://reviews.apache.org/r/30950/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 30950: Add the option to make a non-hooked API.
On Feb. 12, 2015, 10:03 p.m., Zameer Manji wrote: src/test/python/apache/aurora/client/cli/util.py, line 313 https://reviews.apache.org/r/30950/diff/1/?file=862321#file862321line313 Why don't you just use the imported values instead of declaring them here again? Just trying to limit the scope of the changes. I didn't want to duplicate these values up at a higher level so I moved them, but I also don't want to change every test that references these values where they currently live. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30950/#review72249 --- On Feb. 12, 2015, 9:59 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30950/ --- (Updated Feb. 12, 2015, 9:59 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1120 https://issues.apache.org/jira/browse/AURORA-1120 Repository: aurora Description --- Add the option to make a non-hooked API. Diffs - src/main/python/apache/aurora/client/cli/context.py f8b289938201da1edd7d6dde2c65124b80b58adb src/main/python/apache/aurora/client/factory.py 85a1398106a575c1b7759b904b32db6b07da7c1b src/test/python/apache/aurora/client/BUILD c55adfe9825b77f418e41fa9a4ba43926bd991ed src/test/python/apache/aurora/client/cli/BUILD 7319962b285b449b275196eb0c4033e963579d8e src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 12360c64a6c6f46d5b96f604c002b59d0f1a9b0e src/test/python/apache/aurora/client/test_factory.py PRE-CREATION src/test/python/apache/aurora/client/util.py PRE-CREATION Diff: https://reviews.apache.org/r/30950/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 31123: Enable checkstyle indentation check, fix violations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31123/#review72796 --- Ship it! Ship It! - Joshua Cohen On Feb. 17, 2015, 6 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31123/ --- (Updated Feb. 17, 2015, 6 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Repository: aurora Description --- This is the first step of many to align our configuration with checkstyle's Google style config. There are enough differences that a full switch will touch every file in our code base, so i'd like to start with targeted changes. Diffs - config/checkstyle/checkstyle.xml c4c6c4457c879fb4becedfff51847ec840df9300 src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b5a3140e3560f790d1db496dca3c2ee0dc96a195 src/main/java/org/apache/aurora/scheduler/http/StructDump.java 12b28be5a5c711edeed10d8e570e3d61cf9ccfbf src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java eae79d59b445ea58f46dc9e3107c03fbd83b6a95 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 9158271f255eb5c04a1394bf4d470875e8e07b45 src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java cb870f9a127b97d3274d07765a65bcb3eaa40dea src/main/java/org/apache/aurora/scheduler/storage/SnapshotStore.java 49584a9e8158f80a45f2df34f917d4bf2229c87b src/main/java/org/apache/aurora/scheduler/storage/Storage.java 2055e11149ca18180a5f21a36e8c16099f0efa49 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 1b7985084d227ef09d483665ae934540a1512ef4 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2a9d36ab2c01960dc5384fc3ed90df4e11c0b12a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 5f08d00d39f016af9bc296e517ad49b66ab5a8de src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java bc26c4c742a7d2e022edae79e1b06f1982a27764 Diff: https://reviews.apache.org/r/31123/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31123: Enable checkstyle indentation check, fix violations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31123/#review72801 --- Ship it! Ship It! - Joshua Cohen On Feb. 17, 2015, 9:49 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31123/ --- (Updated Feb. 17, 2015, 9:49 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Repository: aurora Description --- This is the first step of many to align our configuration with checkstyle's Google style config. There are enough differences that a full switch will touch every file in our code base, so i'd like to start with targeted changes. Diffs - config/checkstyle/checkstyle.xml c4c6c4457c879fb4becedfff51847ec840df9300 src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b5a3140e3560f790d1db496dca3c2ee0dc96a195 src/main/java/org/apache/aurora/scheduler/http/StructDump.java 12b28be5a5c711edeed10d8e570e3d61cf9ccfbf src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java eae79d59b445ea58f46dc9e3107c03fbd83b6a95 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 9158271f255eb5c04a1394bf4d470875e8e07b45 src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java cb870f9a127b97d3274d07765a65bcb3eaa40dea src/main/java/org/apache/aurora/scheduler/storage/SnapshotStore.java 49584a9e8158f80a45f2df34f917d4bf2229c87b src/main/java/org/apache/aurora/scheduler/storage/Storage.java 2055e11149ca18180a5f21a36e8c16099f0efa49 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 1b7985084d227ef09d483665ae934540a1512ef4 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7014d598d41f007b0dee0b2db1aa2d4cdd592be6 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java d0e11932e8b5ba1393279137c8465a308e1d6bf5 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 5f08d00d39f016af9bc296e517ad49b66ab5a8de src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java bc26c4c742a7d2e022edae79e1b06f1982a27764 Diff: https://reviews.apache.org/r/31123/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review70076 --- Ship it! Looks reasonable to me (caveat being I'm not super familiar with the internals of the scheduler driven updates). - Joshua Cohen On Jan. 23, 2015, 8:37 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Jan. 23, 2015, 8:37 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). Not particularly happy with plumbing through OneWayJobUpdater but the alternative is a state machine change, which is much hairier and will require more changes in the JobUpdaterController. Going with the minimal diff here. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java a992938d4e12b20f81608be6bbdc24c0a211c3fd src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 4c827b183a87b4d97774edbfaa960bd1c3de72a5 src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70244 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/30389/#comment115321 Should we include the proper way to update a non-service job (kill/create) in this message? - Joshua Cohen On Jan. 29, 2015, 5:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 5:13 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30446: Fix compile errors under Java 8.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30446/ --- (Updated Jan. 30, 2015, 6:18 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description (updated) --- Fix compile errors under Java 8. I think this is just caused by the Java 8 compiler having better type inference than Java 7? Diffs - src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 97ecb742d6e0418890f875394ded8d9fdae2b1c2 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f96110c55011699768f17cc2d4afdc8bf7daa16c src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 4ed6b159afda3f118e8ae28d03fdf796cbd98149 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java 131bd826dfe47f40f3c27f29c095ed42953e316c src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 6eaf3ce765c8e50b6724e40848ceb9105e1ab529 src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java 88fc172be6c24fefb6f708ce757487082542 src/test/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterStateTest.java 763f8b5f31349f291c0ede7b5d3292f6ca5b src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 4c2448ffba5c7e0f0ea59fc6484fbcdfc7df7f52 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/http/MnameTest.java afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5 src/test/java/org/apache/aurora/scheduler/mesos/SchedulerDriverServiceTest.java 09593b15c9bd711530ddcb5508ed85b58a2ebe02 src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java cb98834e925793fc116814371548a30470830164 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595 src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java d492e176e73cbd2b3c696fc488010db16106b36a Diff: https://reviews.apache.org/r/30446/diff/ Testing --- ./gradlew build -Pq Thanks, Joshua Cohen
Re: Review Request 30446: Fix compile errors under Java 8.
On Jan. 30, 2015, 4:56 p.m., Maxim Khutornenko wrote: I'm not in love with this fix, but the alternative is to declare these methods as throwing Exception which propagates out pretty widely. - that happens to be exactly what we do in java unit tests. No matter the type, it will still surface in the run log. Ok, incoming large diff ;). - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30446/#review70374 --- On Jan. 30, 2015, 7:04 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30446/ --- (Updated Jan. 30, 2015, 7:04 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Fix compile errors under Java 8. I think this is just caused by the Java 8 compiler having better type inference than Java 7? I'm not in love with this fix, but the alternative is to declare these methods as throwing `Exception` which propagates out pretty widely. Happy to go that route if folks prefer, but figured I'd start with the more tactical fix. Diffs - src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java d492e176e73cbd2b3c696fc488010db16106b36a Diff: https://reviews.apache.org/r/30446/diff/ Testing --- ./gradlew build -Pq Thanks, Joshua Cohen
Re: Review Request 30446: Fix compile errors under Java 8.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30446/ --- (Updated Jan. 30, 2015, 6:18 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Propagate exceptions throughout all tests. Repository: aurora Description --- Fix compile errors under Java 8. I think this is just caused by the Java 8 compiler having better type inference than Java 7? I'm not in love with this fix, but the alternative is to declare these methods as throwing `Exception` which propagates out pretty widely. Happy to go that route if folks prefer, but figured I'd start with the more tactical fix. Diffs (updated) - src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 97ecb742d6e0418890f875394ded8d9fdae2b1c2 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f96110c55011699768f17cc2d4afdc8bf7daa16c src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 4ed6b159afda3f118e8ae28d03fdf796cbd98149 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java 131bd826dfe47f40f3c27f29c095ed42953e316c src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 6eaf3ce765c8e50b6724e40848ceb9105e1ab529 src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java 88fc172be6c24fefb6f708ce757487082542 src/test/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterStateTest.java 763f8b5f31349f291c0ede7b5d3292f6ca5b src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 4c2448ffba5c7e0f0ea59fc6484fbcdfc7df7f52 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/http/MnameTest.java afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5 src/test/java/org/apache/aurora/scheduler/mesos/SchedulerDriverServiceTest.java 09593b15c9bd711530ddcb5508ed85b58a2ebe02 src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java cb98834e925793fc116814371548a30470830164 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595 src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java d492e176e73cbd2b3c696fc488010db16106b36a Diff: https://reviews.apache.org/r/30446/diff/ Testing --- ./gradlew build -Pq Thanks, Joshua Cohen
Re: Review Request 30249: Add CONTRIBUTING.md so github shows a link to it before opening a PR
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/#review69552 --- I think this is a great idea, it'll definitely help point people to ReviewBoard rather than opening pull requests. That said, as far as I can tell, Github has no special processing for including one markdown doc within another, so presumably this will render literally as the string docs/contributing.md. Can you add a little bit more content to this file so that it says something like, Please see [the contributing guidelines](docs/contributing.md) for details on how to contribute patches to Aurora. - Joshua Cohen On Jan. 24, 2015, 9:53 p.m., Jeffrey Schroeder wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30249/ --- (Updated Jan. 24, 2015, 9:53 p.m.) Review request for Aurora. Repository: aurora Description --- Add CONTRIBUTING.md so github shows a link to it before opening a PR Diffs - CONTRIBUTING.md PRE-CREATION Diff: https://reviews.apache.org/r/30249/diff/ Testing --- I accidentally opened a github pull request to fix a small documentation tyop previously. Per the [github documentation](https://github.com/blog/1184-contributing-guidelines), if you have CONTRIBUTING.md in the root of the project, it will be shown before a user ever opens a pull request. Probably makes sense to prevent people from opening pull requests in the future. Note that this is simply a symlink to docs/contributing.md. Thanks, Jeffrey Schroeder
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review69834 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java https://reviews.apache.org/r/30225/#comment114636 Given how sensitive we are to storage lock contention, is there any work in this block that can be done outside of the lock? From a casual glance the only potentially time-consuming (non-write) operation is the fetch from the job update store, but I'm not sure how intensive an operation that will be (or the impact of performing it outside of the context of a lock). - Joshua Cohen On Jan. 23, 2015, 8:37 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Jan. 23, 2015, 8:37 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). Not particularly happy with plumbing through OneWayJobUpdater but the alternative is a state machine change, which is much hairier and will require more changes in the JobUpdaterController. Going with the minimal diff here. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java a992938d4e12b20f81608be6bbdc24c0a211c3fd src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 4c827b183a87b4d97774edbfaa960bd1c3de72a5 src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30446: Fix compile errors under Java 8.
On Jan. 30, 2015, 6:44 p.m., Bill Farner wrote:
I was able to address these compiler errors with a more targeted change,
adding a type witness:
```
$ git diff
src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
diff --git
a/src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
b/src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
index d492e17..6feddc3 100644
---
a/src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
+++
b/src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
@@ -75,7 +75,7 @@ public class StorageTestUtil {
public T IExpectationSettersT expectRead() {
final CaptureWorkT, RuntimeException work =
EasyMockTest.createCapture();
-return expect(storage.read(capture(work)))
+return expect(storage.T, RuntimeExceptionread(capture(work)))
.andAnswer(new IAnswerT() {
@Override
public T answer() {
@@ -86,7 +86,7 @@ public class StorageTestUtil {
public T IExpectationSettersT expectWrite() {
final CaptureMutateWorkT, RuntimeException work =
EasyMockTest.createCapture();
-return expect(storage.write(capture(work))).andAnswer(new IAnswerT()
{
+return expect(storage.T,
RuntimeExceptionwrite(capture(work))).andAnswer(new IAnswerT() {
@Override
public T answer() {
return work.getValue().apply(mutableStoreProvider);
```
Thank you, I knew there had to be a cleaner way to do this! Will send updated
diff.
- Joshua
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30446/#review70393
---
On Jan. 30, 2015, 6:18 p.m., Joshua Cohen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30446/
---
(Updated Jan. 30, 2015, 6:18 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Repository: aurora
Description
---
Fix compile errors under Java 8.
I think this is just caused by the Java 8 compiler having better type
inference than Java 7?
Diffs
-
src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java
97ecb742d6e0418890f875394ded8d9fdae2b1c2
src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java
4e7efb3c1214c3d193afd61f162713490eb8effb
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
f96110c55011699768f17cc2d4afdc8bf7daa16c
src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java
4ed6b159afda3f118e8ae28d03fdf796cbd98149
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java
422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java
131bd826dfe47f40f3c27f29c095ed42953e316c
src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java
6eaf3ce765c8e50b6724e40848ceb9105e1ab529
src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java
88fc172be6c24fefb6f708ce757487082542
src/test/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterStateTest.java
763f8b5f31349f291c0ede7b5d3292f6ca5b
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
4c2448ffba5c7e0f0ea59fc6484fbcdfc7df7f52
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java
020b67187a18bba64d9b562c3a6c0969fc85d469
src/test/java/org/apache/aurora/scheduler/http/MnameTest.java
afa83f2c6f8b03fbd869c8e5c4d81b27a0cc5be5
src/test/java/org/apache/aurora/scheduler/mesos/SchedulerDriverServiceTest.java
09593b15c9bd711530ddcb5508ed85b58a2ebe02
src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java
cb98834e925793fc116814371548a30470830164
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
0a5cc51967f756411ca1489d81872f863c045b6b
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
8fc3cb865fbcd467db91f4cb828d381a02ba7595
src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
d492e176e73cbd2b3c696fc488010db16106b36a
Diff: https://reviews.apache.org/r/30446/diff/
Testing
---
./gradlew build -Pq
Thanks,
Joshua Cohen
Re: Review Request 30446: Fix compile errors under Java 8.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30446/ --- (Updated Jan. 30, 2015, 6:53 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Cleaner fix, thanks Bill! Repository: aurora Description --- Fix compile errors under Java 8. I think this is just caused by the Java 8 compiler having better type inference than Java 7? Diffs (updated) - src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java d492e176e73cbd2b3c696fc488010db16106b36a Diff: https://reviews.apache.org/r/30446/diff/ Testing --- ./gradlew build -Pq Thanks, Joshua Cohen
Re: Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 10, 2015, 12:44 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- *sigh* Include that target in :all while I'm at it. Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs (updated) - examples/vagrant/provision-dev-cluster.sh 490a8194424d4adddc2d9b3a7a7f0e579bea6c06 src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/test/python/apache/aurora/client/cli/BUILD 407cda9a374c37ec3905e09c2a51f3737a335eec src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION src/test/sh/org/apache/aurora/e2e/test_common.sh 31646425233470b5f87ab50ef4504264f235f48a src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 578662ccd1735ebf500d066b3cc17b30f635c15f Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 10, 2015, 12:42 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Add test target for test_version. derp. Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs (updated) - examples/vagrant/provision-dev-cluster.sh 490a8194424d4adddc2d9b3a7a7f0e579bea6c06 src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/test/python/apache/aurora/client/cli/BUILD 407cda9a374c37ec3905e09c2a51f3737a335eec src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION src/test/sh/org/apache/aurora/e2e/test_common.sh 31646425233470b5f87ab50ef4504264f235f48a src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 578662ccd1735ebf500d066b3cc17b30f635c15f Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 10, 2015, 12:04 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- - Fix end to end tests to rsync symlinks - Add end to end test for the output of aurora --version - Fix end to end test cleanup to use v2 syntax Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs (updated) - examples/vagrant/provision-dev-cluster.sh 490a8194424d4adddc2d9b3a7a7f0e579bea6c06 src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION src/test/sh/org/apache/aurora/e2e/test_common.sh 31646425233470b5f87ab50ef4504264f235f48a src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 578662ccd1735ebf500d066b3cc17b30f635c15f Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/#review67685 --- @ReviewBot retry - Joshua Cohen On Jan. 12, 2015, 6:56 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 12, 2015, 6:56 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs - .auroraversion e8ff9d45c6326c28dae836a1409ec0c9b00fd75a .auroraversion PRE-CREATION BUILD 992f6750a97a4f0224dbfb8d4ecc5abeb976af69 examples/vagrant/provision-dev-cluster.sh 490a8194424d4adddc2d9b3a7a7f0e579bea6c06 src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/main/resources/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/resources/apache/aurora/client/cli/BUILD PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD 407cda9a374c37ec3905e09c2a51f3737a335eec src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION src/test/sh/org/apache/aurora/e2e/test_common.sh 31646425233470b5f87ab50ef4504264f235f48a src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 578662ccd1735ebf500d066b3cc17b30f635c15f Diff: https://reviews.apache.org/r/29770/diff/ Testing --- Ran on my (mac) laptop and within the vagrant image: ./pants build src/test/python:all ./gradlew clean build Also ran e2e tests: bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 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/#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/ --- (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
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/ --- (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
Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs - src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 5060577daec0f643f55d8b34141607bb65696559 src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Review Request 29772: Add build-support/rbtools to .gitignore
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29772/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- Add build-support/rbtools to .gitignore Diffs - .gitignore 86840972c53ea52a793968d3d00df6763a7d6ffb Diff: https://reviews.apache.org/r/29772/diff/ Testing --- $ git status Thanks, Joshua Cohen
Re: Review Request 29772: Add build-support/rbtools to .gitignore
On Jan. 9, 2015, 7:06 p.m., Kevin Sweeney wrote: .gitignore, line 18 https://reviews.apache.org/r/29772/diff/1/?file=814816#file814816line18 I don't see this in my repo - is there some more context for this change? Bill Farner wrote: Ditto. I had assumed everyone was just living with this! I just `git clean -fdx`'d and I can't seem to get it to come back. I'l discard this review for now. If/when it shows back up I'll see what caused it and fix the source. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29772/#review67483 --- On Jan. 9, 2015, 6:47 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29772/ --- (Updated Jan. 9, 2015, 6:47 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- Add build-support/rbtools to .gitignore Diffs - .gitignore 86840972c53ea52a793968d3d00df6763a7d6ffb Diff: https://reviews.apache.org/r/29772/diff/ Testing --- $ git status Thanks, Joshua Cohen
Re: Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 9, 2015, 7:05 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- rebase Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs (updated) - src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 9, 2015, 7:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Use \_\_version__ Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs (updated) - src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 29774: Scrub docs of remaining references to aurora2 and aurora help.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29774/#review67487 --- Ship it! docs/developing-aurora-client.md https://reviews.apache.org/r/29774/#comment111530 webUI here is inconsistent with web-ui on the previous line. Either one works, but it should be consistent. - Joshua Cohen On Jan. 9, 2015, 7:12 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29774/ --- (Updated Jan. 9, 2015, 7:12 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- I've also remove some big chunks that are either superfluous, redundant, or outdated. Diffs - docs/cron-jobs.md c3e8eede307fa94807bd65b34d9c24711c8269c1 docs/developing-aurora-client.md 83b843efad3b8dab262c0e4102c5e94d25e73d38 docs/tutorial.md e46addd664ae6219b10b6a57822efeabf16ede40 docs/user-guide.md 877f5b7f44e58dd102a6111635e1b669ef24508f src/test/sh/org/apache/aurora/e2e/test_run.sh c3822addd0f28b5b19210e68ce74e60f0f3fd856 Diff: https://reviews.apache.org/r/29774/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 29770: Add support for --version flag to client.
On Jan. 9, 2015, 7:23 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/__init__.py, lines 228-230 https://reviews.apache.org/r/29770/diff/2/?file=814850#file814850line228 Can this be a module-level constant like ```py __version__ = pkg_resources.resource_string(__name__, '.auroraversion') ``` in accordance with the Python convention? Done. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/#review67488 --- On Jan. 9, 2015, 7:38 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 9, 2015, 7:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs - src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION Diff: https://reviews.apache.org/r/29770/diff/ Testing --- ./pants build src/test/python/apache/aurora/client:: Thanks, Joshua Cohen
Re: Review Request 29774: Scrub docs of remaining references to aurora2 and aurora help.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29774/#review67514 --- Ship it! Ship It! - Joshua Cohen On Jan. 9, 2015, 8:51 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29774/ --- (Updated Jan. 9, 2015, 8:51 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- I've also removed some big chunks that are either superfluous, redundant, or outdated. Diffs - docs/cron-jobs.md c3e8eede307fa94807bd65b34d9c24711c8269c1 docs/developing-aurora-client.md 83b843efad3b8dab262c0e4102c5e94d25e73d38 docs/tutorial.md e46addd664ae6219b10b6a57822efeabf16ede40 docs/user-guide.md 877f5b7f44e58dd102a6111635e1b669ef24508f src/test/sh/org/apache/aurora/e2e/test_run.sh c3822addd0f28b5b19210e68ce74e60f0f3fd856 Diff: https://reviews.apache.org/r/29774/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 29586: Replace twitter.common.python dependency with a direct pex dependency (at the latest version).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/#review66909 --- *ping* - Joshua Cohen On Jan. 5, 2015, 7:07 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/ --- (Updated Jan. 5, 2015, 7:07 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Replace twitter.common.python dependency with a direct pex dependency (at the latest version). Diffs - 3rdparty/python/requirements.txt 05bbf338196cced9f01bf1fadf65682e14dbdf99 src/main/python/apache/aurora/client/cli/BUILD e61cdfb5f3370ac1c5069632d4158f5ee641bc3a src/main/python/apache/aurora/client/commands/BUILD 78a2f57b4b42edf363f40e2988cf9a69c36ad003 src/main/python/apache/aurora/common/BUILD 1c6464d8a91a84ca74191814edacaac5e83b78e8 src/main/python/apache/aurora/common/pex_version.py 6aecd8a14eff7cd58becbecc8b05ea193a6c9cec src/main/python/apache/aurora/executor/BUILD 72d1ec5e891a4b7b5101ae913f7520609ccd98a8 src/main/python/apache/aurora/executor/executor_vars.py 7c018271724ffab2ff6930e5802a48b50a39dded src/test/python/apache/aurora/common/test_pex_version.py 7280f703463c6205493a718310f20a7fd21a0c6b Diff: https://reviews.apache.org/r/29586/diff/ Testing --- ./pants build src/test/python/apache/aurora:all bash src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
Re: Review Request 29828: Patch ResourceManager into OSS Aurora.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29828/#review67744 --- src/main/python/apache/aurora/executor/bin/thermos_executor_main.py https://reviews.apache.org/r/29828/#comment111832 nit: should this be 2 spaces, not 4? src/main/python/apache/aurora/executor/common/resource_manager.py https://reviews.apache.org/r/29828/#comment111831 Add license header to all these new files. - Joshua Cohen On Jan. 12, 2015, 11:25 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29828/ --- (Updated Jan. 12, 2015, 11:25 p.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Bugs: AURORA-1002 https://issues.apache.org/jira/browse/AURORA-1002 Repository: aurora Description --- Patch ResourceManager into OSS Aurora. Diffs - src/main/python/apache/aurora/executor/bin/BUILD 0434c7ba480a80a9722e626775cf5c3adbc3e68e src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 src/main/python/apache/aurora/executor/common/BUILD 142ec0d42b2cb31dcef1e32768e9ea5286355913 src/main/python/apache/aurora/executor/common/resource_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/BUILD 2bf6b2df4761acaa88c38868f03e686ec6b42ab7 src/test/python/apache/aurora/executor/common/test_resource_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION Diff: https://reviews.apache.org/r/29828/diff/ Testing --- Ran e2e v1 tests which broke, but not because of the RM wiring. Running e2e2 tests now. Thanks, Brian Wickman
Re: Review Request 29828: Patch ResourceManager into OSS Aurora.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29828/#review67749 --- Ship it! Ship It! - Joshua Cohen On Jan. 12, 2015, 11:40 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29828/ --- (Updated Jan. 12, 2015, 11:40 p.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Bugs: AURORA-1002 https://issues.apache.org/jira/browse/AURORA-1002 Repository: aurora Description --- Patch ResourceManager into OSS Aurora. Diffs - src/main/python/apache/aurora/executor/bin/BUILD 0434c7ba480a80a9722e626775cf5c3adbc3e68e src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 src/main/python/apache/aurora/executor/common/BUILD 142ec0d42b2cb31dcef1e32768e9ea5286355913 src/main/python/apache/aurora/executor/common/resource_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/BUILD 2bf6b2df4761acaa88c38868f03e686ec6b42ab7 src/test/python/apache/aurora/executor/common/test_resource_manager.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION Diff: https://reviews.apache.org/r/29828/diff/ Testing --- Ran e2e v1 tests which broke, but not because of the RM wiring. Running e2e2 tests now. Thanks, Brian Wickman
Re: Review Request 29829: Fixed cleanup in end to end failure after v1 client removal
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29829/#review67752 --- I've actually included this in my changes for the --version flag: https://reviews.apache.org/r/29770/diff/# - Joshua Cohen On Jan. 12, 2015, 11:44 p.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29829/ --- (Updated Jan. 12, 2015, 11:44 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Fixed cleanup in end to end failure after v1 client removal Diffs - src/test/sh/org/apache/aurora/e2e/test_common.sh 31646425233470b5f87ab50ef4504264f235f48a Diff: https://reviews.apache.org/r/29829/diff/ Testing --- Thanks, Steve Niemitz
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/ --- (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 29770: Add support for --version flag to client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29770/ --- (Updated Jan. 12, 2015, 6:56 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Move .auroraversion under src/main/resources to work around platform inconsistencies in pants when dealing w/ symlinks. Bugs: AURORA-970 and AURORA-989 https://issues.apache.org/jira/browse/AURORA-970 https://issues.apache.org/jira/browse/AURORA-989 Repository: aurora Description --- Add support for --version flag to client. Right now it just prints out the version from .auroraversion. We no longer print out the build sha/date from PEX_INFO. The one wonky thing here is that in order to read .auroraversion from the client, I'm symlinking it under src/main/aurora/apache/client/cli. Open to better solutions if anyone can think of something. Diffs (updated) - .auroraversion e8ff9d45c6326c28dae836a1409ec0c9b00fd75a .auroraversion PRE-CREATION BUILD 992f6750a97a4f0224dbfb8d4ecc5abeb976af69 examples/vagrant/provision-dev-cluster.sh 490a8194424d4adddc2d9b3a7a7f0e579bea6c06 src/main/python/apache/aurora/client/cli/BUILD c7ca61dc5ffc18c95820d38d55228ffad58412ea src/main/python/apache/aurora/client/cli/__init__.py 29998833b50cca2c10eb5c248de9ee1cb60c7a5c src/main/resources/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/resources/apache/aurora/client/cli/BUILD PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD 407cda9a374c37ec3905e09c2a51f3737a335eec src/test/python/apache/aurora/client/cli/test_version.py PRE-CREATION src/test/sh/org/apache/aurora/e2e/test_common.sh 31646425233470b5f87ab50ef4504264f235f48a src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 578662ccd1735ebf500d066b3cc17b30f635c15f Diff: https://reviews.apache.org/r/29770/diff/ Testing (updated) --- Ran on my (mac) laptop and within the vagrant image: ./pants build src/test/python:all ./gradlew clean build Also ran e2e tests: bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
Review Request 30818: Support separate routes for job controller tabs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/ --- Review request for Aurora and David McLaughlin. Bugs: AURORA-696 https://issues.apache.org/jira/browse/AURORA-696 Repository: aurora Description --- Previously navigating between the active/completed/all tabs on the job controller page did not alter the browser history. Now it does, meaning that you can navigate with the back button as well as link directly to a tab. Diffs - src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd Diff: https://reviews.apache.org/r/30818/diff/ Testing --- ./gradlew jshint Verified push state worked in vagrant. Thanks, Joshua Cohen
Re: Review Request 30818: Support separate routes for job controller tabs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/ --- (Updated Feb. 10, 2015, 6:01 a.m.) Review request for Aurora and David McLaughlin. Changes --- Checkstyle. Apparently it's jsHint, not jshint. Gradle didn't complain about the latter being an invalid task and reported everything as ok. Bugs: AURORA-696 https://issues.apache.org/jira/browse/AURORA-696 Repository: aurora Description --- Previously navigating between the active/completed/all tabs on the job controller page did not alter the browser history. Now it does, meaning that you can navigate with the back button as well as link directly to a tab. Diffs (updated) - src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd Diff: https://reviews.apache.org/r/30818/diff/ Testing --- ./gradlew jshint Verified push state worked in vagrant. Thanks, Joshua Cohen
Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review71835 --- src/test/python/apache/aurora/executor/common/test_health_checker.py https://reviews.apache.org/r/30647/#comment117703 Why do we need these real timeouts? - Joshua Cohen On Feb. 6, 2015, 11:13 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 6, 2015, 11:13 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py def249c2509a28f7145380f250f79202b653dc83 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Re: Review Request 30681: docs: Expand Getting Started document
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30681/#review71841 --- Ship it! Ship It! - Joshua Cohen On Feb. 6, 2015, 5:07 p.m., Ricardo Cervera-Navarro wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30681/ --- (Updated Feb. 6, 2015, 5:07 p.m.) Review request for Aurora, Chris Aniszczyk, Dave Lester, Joshua Cohen, Marko Gargenta, and Zameer Manji. Repository: aurora Description --- docs: Expand Getting Started document Diffs - docs/vagrant.md 67f042627bd08632cd237a550153b280e87e324d Diff: https://reviews.apache.org/r/30681/diff/ Testing --- Thanks, Ricardo Cervera-Navarro
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review71840 --- Ship it! (assuming it rebases cleanly without the need for major changes) src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java https://reviews.apache.org/r/30325/#comment117708 I don't know how you feel about the need for a Supplier, but using Optional#or[1] here might read better? [1] http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Optional.html#or(com.google.common.base.Supplier) - Joshua Cohen On Feb. 10, 2015, 12:53 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 10, 2015, 12:53 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30681: docs: Expand Getting Started document
On Feb. 10, 2015, 7:32 p.m., Joshua Cohen wrote: Ship It! Zameer Manji wrote: Don't forget to commit this. I had missed your earlier ship it. Will merge it now. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30681/#review71841 --- On Feb. 6, 2015, 5:07 p.m., Ricardo Cervera-Navarro wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30681/ --- (Updated Feb. 6, 2015, 5:07 p.m.) Review request for Aurora, Chris Aniszczyk, Dave Lester, Joshua Cohen, Marko Gargenta, and Zameer Manji. Repository: aurora Description --- docs: Expand Getting Started document Diffs - docs/vagrant.md 67f042627bd08632cd237a550153b280e87e324d Diff: https://reviews.apache.org/r/30681/diff/ Testing --- Thanks, Ricardo Cervera-Navarro
Re: Review Request 30985: Migrating documentation from v1 commands to v2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30985/#review72423 --- Ship it! Thanks for doing this! Can you just make the minor change below, then I'll commit this. docs/user-guide.md https://reviews.apache.org/r/30985/#comment118483 the syntax for updating a range of shards in v2 is: aurora job update cluster/role/env/job/0-1 - Joshua Cohen On Feb. 13, 2015, 7:32 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30985/ --- (Updated Feb. 13, 2015, 7:32 p.m.) Review request for Aurora and Joshua Cohen. Bugs: AURORA-1113 https://issues.apache.org/jira/browse/AURORA-1113 Repository: aurora Description --- In 0.7.0 the old v1 cli is removed, so a lot of the documentation is outdated. I've updated the v1 commands with their v2 counterparts Diffs - docs/client-commands.md ae752636232f9c354d7011c1c89252e3e747cc9a docs/configuration-reference.md ce59bee302ee2323b65622635b8b51d965dd6507 docs/configuration-tutorial.md 48d7e5dc58faf993bc6ead9796af765fbf7627a4 docs/hooks.md 65e581d795e27fcc6cd683f563f87ee812462598 docs/tutorial.md 7ea36e4ddbe2d57facb86b7dcfc8ff28916de322 docs/user-guide.md 154b47010f739593fd727c1888f1e5cefd63e828 Diff: https://reviews.apache.org/r/30985/diff/ Testing --- Thanks, Florian Pfeiffer
Re: Review Request 30957: Extract ReadOnlyScheduler to its own implementation class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30957/#review72416 --- Ship it! Ship It! - Joshua Cohen On Feb. 13, 2015, 12:55 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30957/ --- (Updated Feb. 13, 2015, 12:55 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- Extract ReadOnlyScheduler to its own implementation class and delegate from the existing SchedulerThriftInterface to it. This enables serving ReadOnlyScheduler from a separate HTTP endpoint (e.g. an unauthenticated one). This is almost a pure tool-driven refactor-rename change. Also renames Util to Responses for improved ergonomics. To boost confidence in this change tests have been left in place. I will follow-up with a subsequent review to split out ReadOnlySchedulerImplTest. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 53ea03bac3784baebc630ad1ce235d263985cafe src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7014d598d41f007b0dee0b2db1aa2d4cdd592be6 src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 7b28eb87767d7cd19e9365e3287f3e943d87dea5 src/main/java/org/apache/aurora/scheduler/thrift/Util.java 55242d18e08ea5cb6dd297bd7f18744d952580b3 src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java e176a0df3141dcb088e05203cca4de3f0d3feea7 src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java cad63c77e8144bb64c6b2acaf1b9199be13e4145 src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java 5e6577d64b1037c69c3a952008240dcafe3b9f94 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 491aa34d76a6f3fe91188bbc73a2cc7464f3644b Diff: https://reviews.apache.org/r/30957/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 30985: Migrating documentation from v1 commands to v2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30985/#review72446 --- Ship it! Ship It! - Joshua Cohen On Feb. 13, 2015, 10:18 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30985/ --- (Updated Feb. 13, 2015, 10:18 p.m.) Review request for Aurora and Joshua Cohen. Bugs: AURORA-1113 https://issues.apache.org/jira/browse/AURORA-1113 Repository: aurora Description --- In 0.7.0 the old v1 cli is removed, so a lot of the documentation is outdated. I've updated the v1 commands with their v2 counterparts Diffs - docs/client-commands.md ae752636232f9c354d7011c1c89252e3e747cc9a docs/configuration-reference.md ce59bee302ee2323b65622635b8b51d965dd6507 docs/configuration-tutorial.md 48d7e5dc58faf993bc6ead9796af765fbf7627a4 docs/hooks.md 65e581d795e27fcc6cd683f563f87ee812462598 docs/tutorial.md 7ea36e4ddbe2d57facb86b7dcfc8ff28916de322 docs/user-guide.md 154b47010f739593fd727c1888f1e5cefd63e828 Diff: https://reviews.apache.org/r/30985/diff/ Testing --- Thanks, Florian Pfeiffer
Re: Review Request 31029: Documenting coordinated updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31029/#review72463 --- Ship it! Just some grammar nits docs/client-commands.md https://reviews.apache.org/r/31029/#comment118531 s/received/are received s/get blocked/be blocked docs/client-commands.md https://reviews.apache.org/r/31029/#comment118532 s/get unblocked/be unblocked docs/configuration-reference.md https://reviews.apache.org/r/31029/#comment118533 s/received/are received s/provided/the provided s/gets blocked/will be blocked - Joshua Cohen On Feb. 13, 2015, 11:25 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31029/ --- (Updated Feb. 13, 2015, 11:25 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1012 https://issues.apache.org/jira/browse/AURORA-1012 Repository: aurora Description --- Documented coordinated updates and added missing UpdateConfig setting definitions. Diffs - docs/client-commands.md ae752636232f9c354d7011c1c89252e3e747cc9a docs/configuration-reference.md ce59bee302ee2323b65622635b8b51d965dd6507 Diff: https://reviews.apache.org/r/31029/diff/ Testing --- https://github.com/maxim111333/incubator-aurora/blob/pulse_docs/docs/client-commands.md#user-content-coordinated-job-updates-beta Thanks, Maxim Khutornenko
Re: Review Request 31022: Fixing aurora beta-update status command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31022/#review72459 --- Ship it! Ship It! - Joshua Cohen On Feb. 13, 2015, 11:27 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31022/ --- (Updated Feb. 13, 2015, 11:27 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1124 https://issues.apache.org/jira/browse/AURORA-1124 Repository: aurora Description --- Fixing aurora beta-update status command. Also, refactored tests a bit and fixed a related issue with aurora `beta-update list` formatting. Diffs - src/main/python/apache/aurora/client/base.py 480728d4c3b574897ea0897f2cc6467af3ba08e6 src/main/python/apache/aurora/client/cli/update.py 6e7e9c64c59ab9a48d016d5bc1e870340f8d5d7e src/test/python/apache/aurora/client/cli/test_supdate.py 0114e200be4d5fb07db855085fce5bc3bc2dded5 src/test/python/apache/aurora/client/test_base.py bc4424b5870ae0c351323bd43d5b38b888f548d5 Diff: https://reviews.apache.org/r/31022/diff/ Testing --- ./pants test.pytest --no-fast src/test/python:all Before: ``` $ aurora beta-update status devcluster/www-data/prod/hello INFO] Command failure: Fatal error running command: Traceback (most recent call last): File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 312, in _execute result = noun.execute(context) File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 390, in execute return self.verbs[context.options.verb].execute(context) File /usr/local/bin/aurora/apache/aurora/client/cli/update.py, line 252, in execute context.log_response_and_raise(response) File /usr/local/bin/aurora/apache/aurora/client/cli/context.py, line 135, in log_response_and_raise self.print_err(\t%s % combine_messages(resp)) File /usr/local/bin/aurora/apache/aurora/client/base.py, line 50, in combine_messages return ', '.join([d.message for d in (response.details or [])]) TypeError: sequence item 0: expected string, NoneType found ``` After: ``` $ aurora beta-update status devcluster/www-data/prod/hello INFO] No updates found for job devcluster/www-data/prod/hello INFO] Command terminated with error code 6 ``` Thanks, Maxim Khutornenko
Re: Review Request 31029: Documenting coordinated updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31029/#review72469 --- Ship it! Ship It! - Joshua Cohen On Feb. 14, 2015, 12:33 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31029/ --- (Updated Feb. 14, 2015, 12:33 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1012 https://issues.apache.org/jira/browse/AURORA-1012 Repository: aurora Description --- Documented coordinated updates and added missing UpdateConfig setting definitions. Diffs - docs/client-commands.md ae752636232f9c354d7011c1c89252e3e747cc9a docs/configuration-reference.md ce59bee302ee2323b65622635b8b51d965dd6507 Diff: https://reviews.apache.org/r/31029/diff/ Testing --- https://github.com/maxim111333/incubator-aurora/blob/pulse_docs/docs/client-commands.md#user-content-coordinated-job-updates-beta Thanks, Maxim Khutornenko
Re: Review Request 30818: Support separate routes for job controller tabs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/ --- (Updated Feb. 10, 2015, 6:01 a.m.) Review request for Aurora and David McLaughlin. Bugs: AURORA-696 https://issues.apache.org/jira/browse/AURORA-696 Repository: aurora Description --- Previously navigating between the active/completed/all tabs on the job controller page did not alter the browser history. Now it does, meaning that you can navigate with the back button as well as link directly to a tab. Diffs - src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd Diff: https://reviews.apache.org/r/30818/diff/ Testing (updated) --- ./gradlew jsHint Verified push state worked in vagrant. Thanks, Joshua Cohen
Re: Review Request 30647: Instrument the HealthChecker to export stats.
On Feb. 5, 2015, 2:32 a.m., Maxim Khutornenko wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 106 https://reviews.apache.org/r/30647/diff/2/?file=849475#file849475line106 Any chance to have test coverage for the other two metrics? +1 - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review71137 --- On Feb. 5, 2015, 1:46 a.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 5, 2015, 1:46 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py def249c2509a28f7145380f250f79202b653dc83 Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review71282 --- @ReviewBot retry - Joshua Cohen On Feb. 5, 2015, 7:17 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 5, 2015, 7:17 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py def249c2509a28f7145380f250f79202b653dc83 Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Review Request 29586: Replace twitter.common.python dependency with a direct pex dependency (at the latest version).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/ --- Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Replace twitter.common.python dependency with a direct pex dependency (at the latest version). Diffs - 3rdparty/python/requirements.txt 05bbf338196cced9f01bf1fadf65682e14dbdf99 src/main/python/apache/aurora/client/cli/BUILD e61cdfb5f3370ac1c5069632d4158f5ee641bc3a src/main/python/apache/aurora/client/commands/BUILD 78a2f57b4b42edf363f40e2988cf9a69c36ad003 src/main/python/apache/aurora/common/BUILD 1c6464d8a91a84ca74191814edacaac5e83b78e8 src/main/python/apache/aurora/common/pex_version.py 6aecd8a14eff7cd58becbecc8b05ea193a6c9cec src/main/python/apache/aurora/executor/BUILD 72d1ec5e891a4b7b5101ae913f7520609ccd98a8 src/main/python/apache/aurora/executor/executor_vars.py 7c018271724ffab2ff6930e5802a48b50a39dded src/test/python/apache/aurora/common/test_pex_version.py 7280f703463c6205493a718310f20a7fd21a0c6b Diff: https://reviews.apache.org/r/29586/diff/ Testing --- ./pants build src/test/python/apache/aurora:all bash src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
Re: Review Request 29586: Replace twitter.common.python dependency with a direct pex dependency (at the latest version).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/ --- (Updated Jan. 5, 2015, 7:07 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Changes --- Sort BUILD dependencies. Repository: aurora Description --- Replace twitter.common.python dependency with a direct pex dependency (at the latest version). Diffs (updated) - 3rdparty/python/requirements.txt 05bbf338196cced9f01bf1fadf65682e14dbdf99 src/main/python/apache/aurora/client/cli/BUILD e61cdfb5f3370ac1c5069632d4158f5ee641bc3a src/main/python/apache/aurora/client/commands/BUILD 78a2f57b4b42edf363f40e2988cf9a69c36ad003 src/main/python/apache/aurora/common/BUILD 1c6464d8a91a84ca74191814edacaac5e83b78e8 src/main/python/apache/aurora/common/pex_version.py 6aecd8a14eff7cd58becbecc8b05ea193a6c9cec src/main/python/apache/aurora/executor/BUILD 72d1ec5e891a4b7b5101ae913f7520609ccd98a8 src/main/python/apache/aurora/executor/executor_vars.py 7c018271724ffab2ff6930e5802a48b50a39dded src/test/python/apache/aurora/common/test_pex_version.py 7280f703463c6205493a718310f20a7fd21a0c6b Diff: https://reviews.apache.org/r/29586/diff/ Testing --- ./pants build src/test/python/apache/aurora:all bash src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
Re: Review Request 29586: Replace twitter.common.python dependency with a direct pex dependency (at the latest version).
On Jan. 5, 2015, 6:53 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/BUILD, line 36 https://reviews.apache.org/r/29586/diff/1/?file=806793#file806793line36 nit: keep these sorted, here and below Done for the rest (this particular one was in the right place afaict). - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/#review66685 --- On Jan. 5, 2015, 7:07 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/ --- (Updated Jan. 5, 2015, 7:07 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Replace twitter.common.python dependency with a direct pex dependency (at the latest version). Diffs - 3rdparty/python/requirements.txt 05bbf338196cced9f01bf1fadf65682e14dbdf99 src/main/python/apache/aurora/client/cli/BUILD e61cdfb5f3370ac1c5069632d4158f5ee641bc3a src/main/python/apache/aurora/client/commands/BUILD 78a2f57b4b42edf363f40e2988cf9a69c36ad003 src/main/python/apache/aurora/common/BUILD 1c6464d8a91a84ca74191814edacaac5e83b78e8 src/main/python/apache/aurora/common/pex_version.py 6aecd8a14eff7cd58becbecc8b05ea193a6c9cec src/main/python/apache/aurora/executor/BUILD 72d1ec5e891a4b7b5101ae913f7520609ccd98a8 src/main/python/apache/aurora/executor/executor_vars.py 7c018271724ffab2ff6930e5802a48b50a39dded src/test/python/apache/aurora/common/test_pex_version.py 7280f703463c6205493a718310f20a7fd21a0c6b Diff: https://reviews.apache.org/r/29586/diff/ Testing --- ./pants build src/test/python/apache/aurora:all bash src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
Re: Review Request 29586: Replace twitter.common.python dependency with a direct pex dependency (at the latest version).
On Jan. 5, 2015, 6:55 p.m., Bill Farner wrote: Can you add details about why this is being done? At a quick glance, it appears as though we're relying on transitive dependencies of pex, but i suspect there's more to it. We were depending on all of twitter.common.python for only pex. Depending on pex directly lets us more easily consume upstream pex changes without going through the dance of getting a new version of t.c.p published. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/#review66687 --- On Jan. 5, 2015, 7:07 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29586/ --- (Updated Jan. 5, 2015, 7:07 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Repository: aurora Description --- Replace twitter.common.python dependency with a direct pex dependency (at the latest version). Diffs - 3rdparty/python/requirements.txt 05bbf338196cced9f01bf1fadf65682e14dbdf99 src/main/python/apache/aurora/client/cli/BUILD e61cdfb5f3370ac1c5069632d4158f5ee641bc3a src/main/python/apache/aurora/client/commands/BUILD 78a2f57b4b42edf363f40e2988cf9a69c36ad003 src/main/python/apache/aurora/common/BUILD 1c6464d8a91a84ca74191814edacaac5e83b78e8 src/main/python/apache/aurora/common/pex_version.py 6aecd8a14eff7cd58becbecc8b05ea193a6c9cec src/main/python/apache/aurora/executor/BUILD 72d1ec5e891a4b7b5101ae913f7520609ccd98a8 src/main/python/apache/aurora/executor/executor_vars.py 7c018271724ffab2ff6930e5802a48b50a39dded src/test/python/apache/aurora/common/test_pex_version.py 7280f703463c6205493a718310f20a7fd21a0c6b Diff: https://reviews.apache.org/r/29586/diff/ Testing --- ./pants build src/test/python/apache/aurora:all bash src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
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/#review66702
---
This is 99% just style nits. Unfortunately our Java styleguide isn't published,
but I'm working on rectifying that!
api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/28920/#comment110279
nit: fix indentation, should be 2 spaces, not 4 (same goes for the Mode
enum below).
examples/jobs/docker/hello_docker.aurora
https://reviews.apache.org/r/28920/#comment110282
indent 2 to be consistent w/ the task below?
examples/jobs/docker/hello_docker.aurora
https://reviews.apache.org/r/28920/#comment110280
put this on a single line?
examples/jobs/docker/hello_docker.aurora
https://reviews.apache.org/r/28920/#comment110281
move to previous line.
examples/vagrant/aurorabuild.sh
https://reviews.apache.org/r/28920/#comment110283
Fix indentation, avoid tabs
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
https://reviews.apache.org/r/28920/#comment110284
Keep indentation consistent with the other args below (indent `help =
...` four spaces).
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
https://reviews.apache.org/r/28920/#comment110286
Insteat of concatenating the strings together just put the help on a
separate line (applies to all instances of this style below).
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
https://reviews.apache.org/r/28920/#comment110288
Do we need to plumb the wrapper path all the way through to the
CommandUtil? What if instead of passing both along we figure out the correct
path to use here at start up and passed that along directly?
I.e. we could do something like...
OptionalString executorPath =
Optional.of(THERMOS_EXECUTOR_PATH.get()).or(Optional.of(THERMOS_EXECUTOR_WRAPPER_PATH.get()));
if (!executorPath.isPresent()) {
throw new IllegalStateException(...);
}
bind(ExecutorSettings.class).toInstance(new ExecutorSettings(
executorPath.get(),
...));
src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
https://reviews.apache.org/r/28920/#comment110302
style nit: method continuation should be formatted like:
public static void create(
String executorUri,
String wrapperUri,
String basePath,
CommandInfo.Builder builder) {
String uriToAdd;
...
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment110303
This could be inlined into the addVolumes call
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment110304
Fits on one line (we use 100 chars as the wrap point).
src/main/python/apache/thermos/config/schema_base.py
https://reviews.apache.org/r/28920/#comment110305
2 blank lines between top level constructs (c.f.
https://www.python.org/dev/peps/pep-0008/#blank-lines).
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/28920/#comment110306
Fix indentation, should be 2 chars.
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/28920/#comment110308
one param per line when exceeding line length:
new ExecutorSettings(
EXECUTOR_PATH,
...);
src/test/python/apache/aurora/executor/test_thermos_executor.py
https://reviews.apache.org/r/28920/#comment110310
This seems like a change in semantics... the sandbox_provider previously
was expected to be a factory function that returned the sandbox, now you're
passing in the sandbox itself? Why the change?
- Joshua Cohen
On Jan. 5, 2015, 8:25 p.m., Steve Niemitz wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28920/
---
(Updated Jan. 5, 2015, 8:25 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
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
Review Request 29674: Fix user agent support for DirectSchedulerClient.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29674/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-988 https://issues.apache.org/jira/browse/AURORA-988 Repository: aurora Description --- Fix user agent support for DirectSchedulerClient. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py a319a1e31f2bb1ab2c18a4ec2b07c35dce545411 src/test/python/apache/aurora/client/api/test_scheduler_client.py a3a40b728d7fb31c681bb684c4613f3ad20c4538 Diff: https://reviews.apache.org/r/29674/diff/ Testing --- ./pants build src/test/python/apache/aurora/client/api:scheduler_client Thanks, Joshua Cohen
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 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 32078: Remove the populatedDEPRECATED thrift field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32078/#review76594 --- Ship it! Ship It! - Joshua Cohen On March 16, 2015, 5:39 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32078/ --- (Updated March 16, 2015, 5:39 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-975 https://issues.apache.org/jira/browse/AURORA-975 Repository: aurora Description --- Remove the populatedDEPRECATED thrift field. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 8386ee36117c8135c7f855a496e1e887904b23dd src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 3876290c3c116ae29a0f386c899ea213efdba318 src/main/python/apache/aurora/client/api/updater.py 7fd6fdacafb7c3d2b30e1d9de2c8a48a88e0e943 src/main/python/apache/aurora/client/base.py 352d3f07148eff3c616f1f2dba861de6a023acc7 src/main/python/apache/aurora/client/cli/jobs.py d6633d93a09f5203219d680cf3780ba1f17d0969 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 3cf8dfde64d38d43e72dd1e7ccc901584b59cbe9 src/test/python/apache/aurora/client/api/test_updater.py 7f3bc28aef52ed171a24ce2a98718afb6bfb1b54 src/test/python/apache/aurora/client/cli/test_diff.py 054e6fbc1b1a56b4818b2a8f08b06e8c64bcfc10 src/test/python/apache/aurora/client/cli/test_restart.py 4f20f6bf3db8d9cae35ec6458777990c763cdbd1 src/test/python/apache/aurora/client/cli/test_update.py 9bfbbea85c6f123076fb570ea91d154dd11c2d78 src/test/python/apache/aurora/client/test_base.py 06c0b436b5104c9b3afbf80499d3bfc66e7ef2f4 Diff: https://reviews.apache.org/r/32078/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32175: Add a test to ensure annotations exist for AuroraSchedulerManager
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32175/#review76828 --- Ship it! Ship It! - Joshua Cohen On March 17, 2015, 9:59 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32175/ --- (Updated March 17, 2015, 9:59 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- This addresses review feedback from https://reviews.apache.org/r/32141/ and prevents divergence when api.thrift changes. Diffs - src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32175/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/#review76787 --- Ship it! src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java https://reviews.apache.org/r/32141/#comment124452 Can you add a corresponding comment to api.thrift that any new methods (or parameters?) added should be added (or updated?) here. - Joshua Cohen On March 17, 2015, 7:41 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/ --- (Updated March 17, 2015, 7:41 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to inherit from it. This gives us a place to put annotations like the AuthorizingParam one introduced in this review without needing to copy-paste them when we override a new method. A future diff will use these annotations to determine which permission a method call needs by inspecting the annotated parameter. I created a new interface to enable DRY - otherwise I'd need to annotate both ForwardingThrift and SchedulerThriftInterface and keep them in sync. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 6b15bfdf727e2db57327936a0341d5dce98bd47c src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java d9184eb540b82c988e7ac590d5cff441f37e62fb src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java f0e40b646092e96955fddc46c3a1e62a8237b00f src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java df6b53a524b005cd5fabb099fd0c08d83e3b287d src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java ee98f66de7f671018fa0a0b4894f114c7a283eda src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 3900c2228038668576cdbb37e87127246a33317c src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 7b1bf2ef8b413d2c1a08b41722a04af091305304 src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java 7e20aaa6836bd205261afe5b1244fb6af8a56356 src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java aae5cd7709abe3896c2ae06c218a0c90ca11c576 Diff: https://reviews.apache.org/r/32141/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.
On March 17, 2015, 7:56 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java, line 39 https://reviews.apache.org/r/32141/diff/5/?file=898006#file898006line39 Can you add a corresponding comment to api.thrift that any new methods (or parameters?) added should be added (or updated?) here. Kevin Sweeney wrote: I'll followup with a test case that ensures all AuroraSchedulerManager.Iface methods are represented here. Even better, thanks! - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/#review76787 --- On March 17, 2015, 8:03 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/ --- (Updated March 17, 2015, 8:03 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to inherit from it. This gives us a place to put annotations like the AuthorizingParam one introduced in this review without needing to copy-paste them when we override a new method. A future diff will use these annotations to determine which permission a method call needs by inspecting the annotated parameter. I created a new interface to enable DRY - otherwise I'd need to annotate both ForwardingThrift and SchedulerThriftInterface and keep them in sync. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 6b15bfdf727e2db57327936a0341d5dce98bd47c src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java d9184eb540b82c988e7ac590d5cff441f37e62fb src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java f0e40b646092e96955fddc46c3a1e62a8237b00f src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java df6b53a524b005cd5fabb099fd0c08d83e3b287d src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java ee98f66de7f671018fa0a0b4894f114c7a283eda src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 3900c2228038668576cdbb37e87127246a33317c src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 7b1bf2ef8b413d2c1a08b41722a04af091305304 src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java 7e20aaa6836bd205261afe5b1244fb6af8a56356 src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java aae5cd7709abe3896c2ae06c218a0c90ca11c576 Diff: https://reviews.apache.org/r/32141/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 32161: Create a packer definition to build a base vagrant box.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32161/#review76786 --- I tried this out locally and `vagrant up` wall time went from 5m24s before this change to 3m16s afterwards. Overall +1 on this, but I definitely think we need to add a README in the packer directory to explain what's going on, how to build/upload a new box, etc. - Joshua Cohen On March 17, 2015, 4:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32161/ --- (Updated March 17, 2015, 4:23 p.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Bugs: AURORA-1203 https://issues.apache.org/jira/browse/AURORA-1203 Repository: aurora Description --- Mostly getting the conversation started. This change pushes most of the lower-velocity components down into a vagrant base box [1], which we would fetch from vagrant cloud for the time being, or on ASF infra if/when freemium ends on Atlas [2]. I managed to an example project that served as a good guide [3]. Getting ahead of potential requests - commenting within packer json is not supported. So far i have moved base package installation out of vagrant provisioning, and have seeded the gradle cache. With a cached machine image (~900 MiB), `vagrant up` took 5m24s (caveat: sample size 1). If we seed the python artifact cache as well, we should be able to shave off another minure or two. [1] http://docs.vagrantup.com/v2/boxes/base.html [2] https://atlas.hashicorp.com/ [3] https://bitbucket.org/ariya/packer-vagrant-linux/src/d0359d7e64e2?at=master This change will not be submitted as-is, since i still need to host the machine image, but i wanted to get your thoughts on it before i proceed further. Diffs - Vagrantfile 2d6c2ae598e80035840f7e517e161be266b581dd examples/packer/aurora.json PRE-CREATION examples/packer/http/ubuntu-14.04-amd64/preseed.cfg PRE-CREATION examples/packer/scripts/aurora.sh PRE-CREATION examples/packer/scripts/compact.sh PRE-CREATION examples/packer/scripts/vagrant.sh PRE-CREATION examples/vagrant/provision-dev-cluster.sh ae500436e703140065e5c16fc0e38dbe3214e69f Diff: https://reviews.apache.org/r/32161/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32319: Add a deprecation warning when using the client-side updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/#review77314 --- Ship it! The updater is dead, long live the updater. - Joshua Cohen On March 20, 2015, 10:27 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/ --- (Updated March 20, 2015, 10:27 p.m.) Review request for Aurora and Joshua Cohen. Bugs: AURORA-1190 https://issues.apache.org/jira/browse/AURORA-1190 Repository: aurora Description --- See summary. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 61337b9312864921508428c7f7992576b94a5d2c src/test/python/apache/aurora/client/cli/test_update.py cc7458568de121dc02de010687fcae0b2707ee0a src/test/python/apache/aurora/client/cli/util.py 864a71428f58ad5ea23beb1d9ae7520c82d2d276 Diff: https://reviews.apache.org/r/32319/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77284 --- Ship it! src/test/python/apache/aurora/client/cli/test_supdate.py https://reviews.apache.org/r/32313/#comment125167 Can we assert `self._mock_api.start_job.mock_calls = [call(...)]` instead? - Joshua Cohen On March 20, 2015, 8:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77291 --- Ship it! src/test/python/apache/aurora/client/cli/test_supdate.py https://reviews.apache.org/r/32313/#comment125174 Yes, let us add stuff! Stuff is great and junk! - Joshua Cohen On March 20, 2015, 10:31 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:31 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32118: Move creation of auth_module into ThriftAuthModule.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32118/#review76629 --- Ship it! Ship It! - Joshua Cohen On March 16, 2015, 6:54 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32118/ --- (Updated March 16, 2015, 6:54 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1201 https://issues.apache.org/jira/browse/AURORA-1201 Repository: aurora Description --- Move creation of auth_module into ThriftAuthModule. Diffs - src/main/java/org/apache/aurora/scheduler/app/Modules.java e86e35f5435e7e83956fd80c8d3ae39eb50c9170 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java c6f8787c894533327114425ba17bca966f07b554 src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java e390af12669ac9464e2e7f6e9e1288136ed5ed9b src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java bb19fc72cafc1640f6331c5eaa3761da1a4af7c0 Diff: https://reviews.apache.org/r/32118/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 7, 2015, 12:01 a.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 200 https://reviews.apache.org/r/31814/diff/1/?file=888014#file888014line200 The original scope for AURORA-1077 was to allow for passing a message along with pause/abort RPCs (so we can, e.g., associate some additional data with a pause caused by an external service monitoring an update). Unless I'm missing something, I don't see that support here. Is that planned for a subsequent review, or has that requirement been missed? Bill Farner wrote: Yes, i'm trying to reasonably scope the reviews to avoid dropping code bombs on reviewers. Thanks, just making sure :). - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75573 --- On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31862: Fix preformatted text when viewed on github
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31862/#review75727 --- README.md https://reviews.apache.org/r/31862/#comment122957 Mind adding `shell` to these blocks? - Joshua Cohen On March 9, 2015, 5:47 p.m., Brian Brazil wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31862/ --- (Updated March 9, 2015, 5:47 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Fix preformatted text when viewed on github Diffs - README.md 6f2731b920fa06a30c557d823ef43d5693759e93 Diff: https://reviews.apache.org/r/31862/diff/ Testing --- Fixed version: https://github.com/brian-brazil/incubator-aurora/blob/readme/README.md Thanks, Brian Brazil
Re: Review Request 31814: Include messages with internal job updater state transitions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75722 --- Ship it! Ship It! - Joshua Cohen On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/#review75723 --- Should we add tests for the full scheduler API as well as the admin interface? Also, worth adding a unit test for the interceptor? src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java https://reviews.apache.org/r/31820/#comment122948 Seems like most of our guice modules have javadoc. Mind adding one here? Or has our style changed in this regard w/ the switch to Google's checkstyle rules? src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java https://reviews.apache.org/r/31820/#comment122950 @CanRead @Exists src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java https://reviews.apache.org/r/31820/#comment122956 javadoc. src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java https://reviews.apache.org/r/31820/#comment122962 What does the last part of these entries represent? The role? Might be worth commenting? - Joshua Cohen On March 7, 2015, 12:54 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/ --- (Updated March 7, 2015, 12:54 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-809 and AURORA-811 https://issues.apache.org/jira/browse/AURORA-809 https://issues.apache.org/jira/browse/AURORA-811 Repository: aurora Description --- * Add dependency on Apache Shiro. * HTTP Basic Authentication. * Authorization based on shiro.ini. * Sample shiro.ini for local mode. Diffs - build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 24b61c1e4f615295acf28d904588e1512972d3f4 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 8a59d89c07b406ce98076ca7ee51b958599a39ec src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 1e4ba014804b56a2ea02770d09beb63faaabf684 src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 640acdf4e73f99418473ca97bcdc4f5f4c190f10 src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 52fe0ea063dbc7a71a20926630bf449dbd936306 src/test/resources/org/apache/aurora/scheduler/app/local/shiro.ini PRE-CREATION Diff: https://reviews.apache.org/r/31820/diff/ Testing --- ./gradlew -Pq build Local testing in the UI and with cURL. Updates to e2e test and Vagrant environment to follow. Thanks, Kevin Sweeney
Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/#review75926 --- Ship it! lgtm! src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java https://reviews.apache.org/r/31820/#comment123255 Maybe move this to the ticket and kill the commented out code here? src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java https://reviews.apache.org/r/31820/#comment123256 nit: probably preferable to move ImmutableSortedSet.of down to the next line. src/main/python/apache/aurora/common/clusters.py https://reviews.apache.org/r/31820/#comment123258 Is this change related? src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31820/#comment123260 Same here, seems unrelated? Does this just need to be rebased? - Joshua Cohen On March 10, 2015, 1:18 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/ --- (Updated March 10, 2015, 1:18 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-809 and AURORA-811 https://issues.apache.org/jira/browse/AURORA-809 https://issues.apache.org/jira/browse/AURORA-811 Repository: aurora Description --- * Add dependency on Apache Shiro. * HTTP Basic Authentication. * Authorization based on shiro.ini. * Sample shiro.ini for local mode. Diffs - build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 24b61c1e4f615295acf28d904588e1512972d3f4 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 8a59d89c07b406ce98076ca7ee51b958599a39ec src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 1e4ba014804b56a2ea02770d09beb63faaabf684 src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 640acdf4e73f99418473ca97bcdc4f5f4c190f10 src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParserTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 52fe0ea063dbc7a71a20926630bf449dbd936306 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-example.ini PRE-CREATION src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-extra-sections.ini PRE-CREATION src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-missing-sections.ini PRE-CREATION Diff: https://reviews.apache.org/r/31820/diff/ Testing --- ./gradlew -Pq build Local testing in the UI and with cURL. Updates to e2e test and Vagrant environment to follow. Thanks, Kevin Sweeney
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 48 https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. David McLaughlin wrote: I think it's better to just give a clear message telling them there is a limit. Truncation could happen in the client if needed. Maxim Khutornenko wrote: I was considering a case when some automated external service would attempt to act on an update and append some arbitrary metadata with pause/resume/abort. While not desirable, does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a higher priority than enforcing a strict audit mode. Bill Farner wrote: Maybe it does warrant a failure, though. IMHO truncation would be a policy decision that the scheduler is making on behalf of the client. If the most important part of the message is after the truncation, we've made a poor choice. Maxim Khutornenko wrote: IDK, this whole length enforcment seems quite arbitrary to me. We don't restrict string size anywhere in thrift API or SQL schema. The only exception is TaskIdGenerator where the ID length is critical for external reasons (MESOS-691). Perhaps we should start doing it everywhere then? If the requirement to restrict message length is not driven by the underlying store, is there any reason to enforce it at all? Could we just let clients truncate messages as appropriate to their use case? Are we concerned with abuse and/or performance impact? - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76094 --- Ship it! LGTM modulo the outstanding question re: length limit. - Joshua Cohen On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32055: Add a flag to configure Shiro at runtime.
On March 13, 2015, 7:59 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java, line 95 https://reviews.apache.org/r/32055/diff/1/?file=894545#file894545line95 I thought it's generally preferable when installing user-supplied modules to do so within a private module to sandbox the module? Kevin Sweeney wrote: it is, but then multibinders don't work: https://groups.google.com/forum/#!topic/google-guice/h70a9pwD6_g Nod, I saw the multibinding but wasn't clear why it was needed now and not in the previous version (answer: previous version also used multibinding, but it was hidden within the bindRealm call). Can you add a comment explaining why you're not doing this in a private module? On March 13, 2015, 7:59 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java, line 15 https://reviews.apache.org/r/32055/diff/1/?file=894547#file894547line15 Do we need this? Can we just use an arg like... Arg? extends Class? extends Module and rely on the existing ClassParser to give us the type, then just call newInstance() on that? Kevin Sweeney wrote: basically this boilerplate will appear everywhere we do this - seems reasonable to standardize on a parser to make the error messages nice. makes unit tests easier as well as we can pass instances of module (usually AICs) instead of a class object. Sounds reasonable! - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32055/#review76410 --- On March 13, 2015, 8:26 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32055/ --- (Updated March 13, 2015, 8:26 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-809 https://issues.apache.org/jira/browse/AURORA-809 Repository: aurora Description --- Add a flag to configure Shiro at runtime. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java 1c3ea24d8d4751f62e1bea260f8823f039e56e10 src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 2876b1f0595fc05fe76718a69ae280b4ce7d7178 src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32055/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 32329: Extract job key from RPC parameters
On March 23, 2015, 8:59 p.m., Joshua Cohen wrote:
Thanks, this is already much easier to follow.
One general question on the overall approach: do you think the DRY benefits
of using composed `StructFieldGetter`s to generate the functions that allow
walking from the starting type to `JobKey` outweigh the readability
improvements of using a fully explicit mapping? I.e.
FunctionJobUpdateRequest, JobKey UPDATE_REQUEST_TO_JOB_KEY = new
Function... {
@Override
public JobKey apply(JobUpdateRequest request) {
return request.getTaskConfig().getJobKey();
}
};
Bill Farner wrote:
I raised this question offline as well. It's not clear to me that this
DRY-ness is worth the complexity. Required null checking in the call chain
is one downside to the more direct approach you illustrated.
Yeah, Kevin and I discussed offline and he mentioned the null checking. If
that's really the only benefit, would it make sense to simply catch the NPE and
return Optional.absent?
- Joshua
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32329/#review77289
---
On March 23, 2015, 7:14 p.m., Kevin Sweeney wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32329/
---
(Updated March 23, 2015, 7:14 p.m.)
Review request for Aurora, Joshua Cohen and Bill Farner.
Bugs: AURORA-1187
https://issues.apache.org/jira/browse/AURORA-1187
Repository: aurora
Description
---
Apologies for the large diff, this wound up needing to input validation at
the AOP layer.
Probably the best place to start reading this diff is ApiSecurityIT to see
the feature this patch enables.
Diffs
-
config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb
src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java
827e85b6cac8bd52359610bbc2002973a769705c
src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
2408cd1f9af5f109a339f5c78134465cb117f7fc
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
cc9cfd38239f909b8a77bd1a773e31ec30130d41
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
808987939b2c4a850e488dc033b50b0178e95ba0
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
4e341e05c34b1be38f0040c26b671a0cc797a771
src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
5588d1793d6713ee4581ac9f938d9a8689acb315
src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
bdd2185f3a7a94b39bcec3c73455e970d87f0c6a
src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java
cafd10f6b705568588c1b92644b482003242fe2e
src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
ed284f46ac8f01bd6d9e317f995f16d6e666a68d
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
76cb691e6d7d4fada3a18fde73aceed7039bcaa4
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
d2ba2730c4509dc9a636fd32e9244b0d7fa2884f
src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
1f24e7d47e1f777ffef19a73d01171fcacd31cdb
src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java
d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b
Diff: https://reviews.apache.org/r/32329/diff/
Testing
---
./gradlew -Pq build
Thanks,
Kevin Sweeney
Review Request 31546: Revert TARGET_PEOPLE change, this was applying to updates as well as new reviews.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31546/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Revert TARGET_PEOPLE change, this was applying to updates as well as new reviews. Diffs - .reviewboardrc 415e2455660a14681f6fedde30f339f9f211e7f5 Diff: https://reviews.apache.org/r/31546/diff/ Testing --- Posted this review. Thanks, Joshua Cohen
Re: Review Request 31659: Clean up end-to-end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31659/#review75048 --- Ship it! Just tried this out locally. So much faster, thanks for doing this! - Joshua Cohen On March 3, 2015, 8:55 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31659/ --- (Updated March 3, 2015, 8:55 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Major changes: - applying DRY by not invoking everything via `vagrant ssh` (making tests easier to write and much faster to run) - pulled 'test cases' into individual functions - simplified (maybe?) setup for tests using SSH This is being done in preparation for AURORA-1157. Diffs - src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 0f6423521ad2c85032b4825a5b9793dc522a3b70 src/test/sh/org/apache/aurora/e2e/http/http_example_docker.aurora 00fa2fb6c272fe01700c4696f92713f11d62230b src/test/sh/org/apache/aurora/e2e/http/http_example_docker_updated.aurora d987d637da4061cd7946074243daabee4b9a150f src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 072bbb76707e6696815509f21d50b6b5446241b3 src/test/sh/org/apache/aurora/e2e/test_common.sh b621975bab6f8bbf193e61360654e34337e36943 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 1d599c3f71b7e07c1cb8bee01a4d3b5cfcf92631 src/test/sh/org/apache/aurora/e2e/test_run.sh ab91b95a60f7b459b95dc7e13a29692badca5ff7 Diff: https://reviews.apache.org/r/31659/diff/ Testing --- Ran the script. Thanks, Bill Farner
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73603 --- Ship it! Ship It! - Joshua Cohen On Feb. 21, 2015, 6:33 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 6:33 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote: Can you fill in testing done? Bill Farner wrote: Honest question - do you find that useful for changes like this? I find it redundant to always type `./gradlew build -Pq`, especially since the build bot will do that anyhow. My take is that, yes, I trust that you ran the tests. But for someone who's pushing a review for the first time, who knows? And if they just pick a random commit from the log and look at the Testing Done section, I'd like for it to be consistent and easy to emulate. On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, lines 199-200 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder and String.format? Bill Farner wrote: See my reply to Maxim's comment. javac is smart about using StringBuilder behind the scenes in cases like this. Do you find String.format more readable in cases like this? Personally i do not. I've never had a problem with multiple concatenated variables, but my main concern is that we're consistent. My interpretation of our style based on review feedback I've gotten is that when concatenating more than one variable into a string we err on the side of using String.format for readability (perhaps we should actually fork the twitter style guide and codify these things?). - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73353 --- On Feb. 21, 2015, 6:33 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 6:33 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
