aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 23254: Refactoring SchedulerCore (killTasks)
Date Mon, 21 Jul 2014 22:16:10 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23254/#review48302
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/23254/#comment84719>

    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.



src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java
<https://reviews.apache.org/r/23254/#comment84717>

    Is this file relevant to the change?



src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
<https://reviews.apache.org/r/23254/#comment84723>

    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?



src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
<https://reviews.apache.org/r/23254/#comment84721>

    Use a constant for string that is otherwise magic.


- Bill Farner


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message