Re: Review Request 23254: Refactoring SchedulerCore (killTasks)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/#review48414 --- Ship it! Ship It! - Kevin Sweeney On July 21, 2014, 4:23 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/ --- (Updated July 21, 2014, 4:23 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Splitting the earlier RB for easier reviewing. Addressing killTasks refactoring. Also moving some useful tests out of BaseSchedulerCoreImplTest. Diffs - src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 27599f75603542069084631baf9195b8ad75e902 src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 628464de0032db4eb5884e3b74346b2390408993 src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b6d06f39ac39570eac4495d97d3adaabe8357bf7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 95ee7b784889d66cae9b714bd88c27a136a54b3a src/main/java/org/apache/aurora/scheduler/thrift/Util.java 4ebb608d28b667bf47573524aa8878ac3b21ecfe src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 8692051fae08a59e4b70b330eb6723f72ff04d43 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2298971ffac45a284f9130e2122aeea8b39dc852 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java d2bd65eef5a8ceea92dd62044e5e2c621c4a4f3e src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 94eb0a22037187625636b24707d4ac6741b55f22 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b28761d5bbfdd2fea4714c79b47be5f5eb3a5280 Diff: https://reviews.apache.org/r/23254/diff/ Testing --- gradle -Pq clean build Thanks, Maxim Khutornenko
Re: Review Request 23254: Refactoring SchedulerCore (killTasks)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/#review48298 --- Ping. - Maxim Khutornenko On July 3, 2014, 1:48 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/ --- (Updated July 3, 2014, 1:48 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Splitting the earlier RB for easier reviewing. Addressing killTasks refactoring. Also moving some useful tests out of BaseSchedulerCoreImplTest. Diffs - src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 27599f75603542069084631baf9195b8ad75e902 src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 628464de0032db4eb5884e3b74346b2390408993 src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b6d06f39ac39570eac4495d97d3adaabe8357bf7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2549dd33d08dfc6058d985127a3f0c1f3984eaa7 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2298971ffac45a284f9130e2122aeea8b39dc852 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java d2bd65eef5a8ceea92dd62044e5e2c621c4a4f3e src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 94eb0a22037187625636b24707d4ac6741b55f22 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2cffa74ba36e2afda3340658d6b1afd6cb50cf2c Diff: https://reviews.apache.org/r/23254/diff/ Testing --- gradle -Pq clean build Thanks, Maxim Khutornenko
Re: Review Request 23254: Refactoring SchedulerCore (killTasks)
On July 21, 2014, 10:16 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 721 https://reviews.apache.org/r/23254/diff/1/?file=623290#file623290line721 We should avoid taking advantage of passing nulls where possible, and in fact i suggest changing addMessage to MorePreconditions.checkNotBlank on that. However, you'll need to take care in cases like addMessage(x, x, exception.getMessage()). For those, it makes sense to add an overload that accepts an Exception instead of String. Makes sense. Done. On July 21, 2014, 10:16 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java, line 1 https://reviews.apache.org/r/23254/diff/1/?file=623294#file623294line1 Is this file relevant to the change? This is related to the Also moving some useful tests out of BaseSchedulerCoreImplTest. part of the changelist. Trying to balance the diff size. On July 21, 2014, 10:16 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, line 431 https://reviews.apache.org/r/23254/diff/1/?file=623295#file623295line431 There's a subtle detail in this test case, that active() is automatically added to the task query. Can you break that out into an independent test case? Sure, done. On July 21, 2014, 10:16 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, line 509 https://reviews.apache.org/r/23254/diff/1/?file=623295#file623295line509 Use a constant for string that is otherwise magic. Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/#review48302 --- On July 3, 2014, 1:48 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/ --- (Updated July 3, 2014, 1:48 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Splitting the earlier RB for easier reviewing. Addressing killTasks refactoring. Also moving some useful tests out of BaseSchedulerCoreImplTest. Diffs - src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 27599f75603542069084631baf9195b8ad75e902 src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 628464de0032db4eb5884e3b74346b2390408993 src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b6d06f39ac39570eac4495d97d3adaabe8357bf7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2549dd33d08dfc6058d985127a3f0c1f3984eaa7 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2298971ffac45a284f9130e2122aeea8b39dc852 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java d2bd65eef5a8ceea92dd62044e5e2c621c4a4f3e src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 94eb0a22037187625636b24707d4ac6741b55f22 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2cffa74ba36e2afda3340658d6b1afd6cb50cf2c Diff: https://reviews.apache.org/r/23254/diff/ Testing --- gradle -Pq clean build Thanks, Maxim Khutornenko
Re: Review Request 23254: Refactoring SchedulerCore (killTasks)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/#review48316 --- Ship it! Ship It! - Bill Farner On July 21, 2014, 11:23 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/ --- (Updated July 21, 2014, 11:23 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Splitting the earlier RB for easier reviewing. Addressing killTasks refactoring. Also moving some useful tests out of BaseSchedulerCoreImplTest. Diffs - src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 27599f75603542069084631baf9195b8ad75e902 src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 628464de0032db4eb5884e3b74346b2390408993 src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b6d06f39ac39570eac4495d97d3adaabe8357bf7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 95ee7b784889d66cae9b714bd88c27a136a54b3a src/main/java/org/apache/aurora/scheduler/thrift/Util.java 4ebb608d28b667bf47573524aa8878ac3b21ecfe src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 8692051fae08a59e4b70b330eb6723f72ff04d43 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2298971ffac45a284f9130e2122aeea8b39dc852 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java d2bd65eef5a8ceea92dd62044e5e2c621c4a4f3e src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 94eb0a22037187625636b24707d4ac6741b55f22 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b28761d5bbfdd2fea4714c79b47be5f5eb3a5280 Diff: https://reviews.apache.org/r/23254/diff/ Testing --- gradle -Pq clean build Thanks, Maxim Khutornenko
Re: Review Request 23254: Refactoring SchedulerCore (killTasks)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/ --- (Updated July 3, 2014, 1:48 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Splitting the earlier RB for easier reviewing. Addressing killTasks refactoring. Also moving some useful tests out of BaseSchedulerCoreImplTest. Diffs - src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 27599f75603542069084631baf9195b8ad75e902 src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 628464de0032db4eb5884e3b74346b2390408993 src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b6d06f39ac39570eac4495d97d3adaabe8357bf7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2549dd33d08dfc6058d985127a3f0c1f3984eaa7 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2298971ffac45a284f9130e2122aeea8b39dc852 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java d2bd65eef5a8ceea92dd62044e5e2c621c4a4f3e src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 94eb0a22037187625636b24707d4ac6741b55f22 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2cffa74ba36e2afda3340658d6b1afd6cb50cf2c Diff: https://reviews.apache.org/r/23254/diff/ Testing --- gradle -Pq clean build Thanks, Maxim Khutornenko
