aurora-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 16220: Output all states in maintenance endpoint.
Date Thu, 12 Dec 2013 23:08:14 GMT

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



src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java
<https://reviews.apache.org/r/16220/#comment57997>

    While N is not _huge_ here, there is redundant work in multiple calls to getHostAttributes().
 I suspect it wouldn't be hard to rework so that's only fetched once.



src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java
<https://reviews.apache.org/r/16220/#comment57990>

    This and the other IS_* functions can be replaced with:
    
    private static Function<HostAttributes, MaintenanceMode> GET_MODE =
        new Function<HostAttributes, MaintenanceMode>() {
          @Override MaintenanceMode apply(HostAttributes attrs) {
            return attr.getMode();
          }
        };
    
    private static Predicate<HostAttributes> hasMode(MaintenanceMode mode) {
      return Predicates.compose(Predicates.equalTo(mode), GET_MODE);
    }
    
    Then inline:
    
    hasMode(DRAINING), etc



src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java
<https://reviews.apache.org/r/16220/#comment57992>

    s/Fluent//



src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java
<https://reviews.apache.org/r/16220/#comment57991>

    this should be only live tasks



src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java
<https://reviews.apache.org/r/16220/#comment57994>

    There's a DRY violation on the snippet of code to get host names by maintenance mode.
 Consider extracting a function:
    
    private static FluentIterable<String> getHostsInMode(MaintenanceMode mode) {
      ...
    }


- Bill Farner


On Dec. 12, 2013, 8:51 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16220/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2013, 8:51 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-9
>     https://issues.apache.org/jira/browse/AURORA-9
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Improve the /maintenance endpoint to print out hosts affected by SCHEDULED and DRAINED
states.
> 
> 
> Diffs
> -----
> 
>   src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java 30afce37d6c108a5a8c1c3c8a8094030ad12ce72

>   src/main/java/com/twitter/aurora/scheduler/state/MaintenanceController.java fb12d38858b260d1d9ce318d3022cd93413a3e68

>   src/test/java/com/twitter/aurora/scheduler/state/MaintenanceControllerImplTest.java
8acb716c733ec9d3cc3b1ec74c85f958082ae139 
> 
> Diff: https://reviews.apache.org/r/16220/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew clean build
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>


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