Re: Review Request 31774: Add aurora-specific entry point for thermos observer and fix /vars
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31774/#review75410 --- Master (70494a1) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On March 5, 2015, 8:31 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31774/ --- (Updated March 5, 2015, 8:31 p.m.) Review request for Aurora, Joe Smith and Zameer Manji. Bugs: AURORA-1133 https://issues.apache.org/jira/browse/AURORA-1133 Repository: aurora Description --- Just like the aurora-specific thermos entry point, adds an aurora-specific thermos_observer entry point. Just allows it to search for thermos runners in /var/lib/thermos and also relative to mesos sandboxes. Diffs - src/main/python/apache/aurora/tools/BUILD 36117b4199bc52873ca3abdb1ef6447437a81bb0 src/main/python/apache/aurora/tools/thermos_observer.py PRE-CREATION src/main/python/apache/thermos/observer/BUILD 28995b99b4991e17eb977583388c89e753055e9b src/main/python/apache/thermos/observer/bin/BUILD a42dbf321b8dcf5d64b22c2a480c3f4e3bad2a42 src/main/python/apache/thermos/observer/bin/thermos_observer.py 213a48eb4e2441b88fd6b608d1f3ba7dd0f2b859 src/main/python/apache/thermos/observer/http/BUILD cc8eb7793b980a6a4b76deece759e12e9bc7fcb0 src/main/python/apache/thermos/observer/http/configure.py PRE-CREATION src/main/python/apache/thermos/observer/http/diagnostics.py PRE-CREATION src/main/python/apache/thermos/observer/http/vars_endpoint.py PRE-CREATION src/main/python/apache/thermos/observer/task_observer.py 6e7517b9f1b70cef8b0400cd7769fbbe7495dc42 Diff: https://reviews.apache.org/r/31774/diff/ Testing --- mba=aurora=; ./pants test.pytest --no-fast src/test3/python:: Thanks, Brian Wickman
Review Request 31779: Change remaining update-related RPCs to use JobUpdateKey.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31779/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1093 https://issues.apache.org/jira/browse/AURORA-1093 Repository: aurora Description --- This was overlooked in AURORA-1093. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 1cd21e598db0c0d51cfed293e5e0fc51d84e0bb0 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8ec5f9a3810b4deae981988487c6a46a20db72a2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 5989a62f1651aede6e2372ad3f519a9a947470de src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/python/apache/aurora/client/api/__init__.py c07122744e89fe61dbe4bea0c14400425983b2ef src/main/python/apache/aurora/client/cli/update.py a4890057b7d258926bc3dfb84fd1248e68051f31 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 824740856236976984d2114ec6a6aea989a87d1e src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 459f745cec1f85ece41376cade39c09254b50013 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/python/apache/aurora/client/api/test_api.py 0d552e8b9f9be300fc28a3f52aabe19e5a51b252 src/test/python/apache/aurora/client/cli/test_create.py a65aab71ee8ce19dc2c05ea230258084d6f55727 src/test/python/apache/aurora/client/cli/test_restart.py b596babc3c6877f68094943126b8cd49be3fc635 src/test/python/apache/aurora/client/cli/test_supdate.py 93a5532dc6f7aee2c40bc86385a630b9a1b6f528 src/test/python/apache/aurora/client/cli/util.py 6d3cc51f50b417405549c254531c854565a54949 Diff: https://reviews.apache.org/r/31779/diff/ Testing --- Normal test suite + end-to-end tests. Thanks, Bill Farner
Re: Review Request 31652: Returning pending reason for all tasks in a group
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/#review75413 --- Ship it! Master (70494a1) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On March 5, 2015, 6:48 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/ --- (Updated March 5, 2015, 6:48 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Modifying `Vetoed` event to broadcast `TaskGroupKey` instead of task ID and storing veto reasons by TaskGroupKey in NearestFit. Depends on https://reviews.apache.org/r/31646/. Diffs - src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java edaf2f4f845544c13b2fb9bc77c34f6e6d96fb48 src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java c103472b9404df1c690b3a6019d64d42e15f2fed src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 53582c63ddee23e643bd4654cad2bef75dfba36d src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 13520eb5846022ed0b43b402096fe02565103aa9 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java ab7817f929bbcc96a6046043ea17921a388fdb9f src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ce5a62650cebab9a53743460f5a5119f62efec1c Diff: https://reviews.apache.org/r/31652/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31753: Add storage support for associating a message with job update events.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31753/#review75405 --- Ship it! Master (70494a1) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On March 5, 2015, 1:19 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31753/ --- (Updated March 5, 2015, 1:19 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Part 1 - storage only. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 1cd21e598db0c0d51cfed293e5e0fc51d84e0bb0 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 1b2aa74ac727a8c0b2cfdbcd8d2a4e23d3bdda63 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml d813b19ab124574cbcc1094cb08bece400fba652 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql a0d6b545acf3d1513dd181499333baee5fed7c97 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 7198db00e065859c85fbdf50de675b26b2f6dd34 Diff: https://reviews.apache.org/r/31753/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31652: Returning pending reason for all tasks in a group
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/ --- (Updated March 5, 2015, 6:48 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Bill's comments. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Modifying `Vetoed` event to broadcast `TaskGroupKey` instead of task ID and storing veto reasons by TaskGroupKey in NearestFit. Depends on https://reviews.apache.org/r/31646/. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java edaf2f4f845544c13b2fb9bc77c34f6e6d96fb48 src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java c103472b9404df1c690b3a6019d64d42e15f2fed src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 53582c63ddee23e643bd4654cad2bef75dfba36d src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 13520eb5846022ed0b43b402096fe02565103aa9 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java ab7817f929bbcc96a6046043ea17921a388fdb9f src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ce5a62650cebab9a53743460f5a5119f62efec1c Diff: https://reviews.apache.org/r/31652/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31652: Returning pending reason for all tasks in a group
On March 3, 2015, 11:56 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java, line 237 https://reviews.apache.org/r/31652/diff/1/?file=882474#file882474line237 s/taskId/groupKey/ Good catch, fixed. On March 3, 2015, 11:56 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 57 https://reviews.apache.org/r/31652/diff/1/?file=882475#file882475line57 Should this cache be renamed? No preference here, renamed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/#review75096 --- On March 3, 2015, 12:58 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/ --- (Updated March 3, 2015, 12:58 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Modifying `Vetoed` event to broadcast `TaskGroupKey` instead of task ID and storing veto reasons by TaskGroupKey in NearestFit. Depends on https://reviews.apache.org/r/31646/. Diffs - src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java edaf2f4f845544c13b2fb9bc77c34f6e6d96fb48 src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java c103472b9404df1c690b3a6019d64d42e15f2fed src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 53582c63ddee23e643bd4654cad2bef75dfba36d src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 13520eb5846022ed0b43b402096fe02565103aa9 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java ab7817f929bbcc96a6046043ea17921a388fdb9f src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ce5a62650cebab9a53743460f5a5119f62efec1c Diff: https://reviews.apache.org/r/31652/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31646: Moving GroupKey to scheduler.base.
On March 3, 2015, 11:54 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java, line 23 https://reviews.apache.org/r/31646/diff/1/?file=882425#file882425line23 s/Identifying/Identifer for/ It's not until reading this diff that i wonder why TaskGroupKey/GroupKey even exists. The best justification i have is separation of concerns from how a task is configured and how it is identified. If you agree, it would be nice to document that here. Done. On March 3, 2015, 11:54 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java, line 38 https://reviews.apache.org/r/31646/diff/1/?file=882425#file882425line38 Why the proxy method to the constructor? To highlight the immutable final purpose of its existence. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31646/#review75093 --- On March 3, 2015, 12:07 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31646/ --- (Updated March 3, 2015, 12:07 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Simple IDE-driven refactoring in preparation for the AURORA-911 fix. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d3713972f1df54da4c8cbaae0fca59a8a6f3d77 src/main/java/org/apache/aurora/scheduler/async/TaskGroup.java e5067e18af7c477d2927d83eacec063f3dec575a src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java 8cd6c966c9aca2e869311787fb5d5caba7439d3e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 0cbc71d50612323aa4d8acc33e74b879c0a76aff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 87bc531d2a72f21c36ddd0c1bd3b2367826cc422 Diff: https://reviews.apache.org/r/31646/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31754: Break out API servlet configuration into its own module.
On March 4, 2015, 7:18 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/app/AppModule.java, line 141 https://reviews.apache.org/r/31754/diff/1/?file=885268#file885268line141 There seems to be implementation detail leaking here. Can you make JettyServerModule hide this away, and force the leak into the unit test instead? Any thoughts on how that should look? Possibly an @VisibleForTesting constructor to JettyServerModule with a boolean production? The test needs to replace the binding for ServletContextListener, which needs a parent Injector handle, since binding a Module isn't allowed due to guice internals. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31754/#review75281 --- On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31754/ --- (Updated March 4, 2015, 5:55 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- Break out API servlet configuration into its own module. This is necessary to make a follow-up patch introducing HTTP Basic auth testable. Diffs - src/main/java/org/apache/aurora/scheduler/app/AppModule.java 5f6a019e4d6401e1efd075b72c049fa245cc0d0a src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 8a59d89c07b406ce98076ca7ee51b958599a39ec src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660 src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6 src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 80beb258d9f2786668d29db85b1295163a402d42 src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 47d54e3c3bb1ba5e0fb26379792f515f25316480 src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 5019094333f9807c64a49c29569ade191ee61824 src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31754/diff/ Testing --- Thanks, Kevin Sweeney
