> On Dec. 12, 2013, 3:08 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java, line 81 > > <https://reviews.apache.org/r/16220/diff/3/?file=396978#file396978line81> > > > > 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
done. > On Dec. 12, 2013, 3:08 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java, line 99 > > <https://reviews.apache.org/r/16220/diff/3/?file=396978#file396978line99> > > > > this should be only live tasks done > On Dec. 12, 2013, 3:08 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java, line 92 > > <https://reviews.apache.org/r/16220/diff/3/?file=396978#file396978line92> > > > > s/Fluent// done > On Dec. 12, 2013, 3:08 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java, line 145 > > <https://reviews.apache.org/r/16220/diff/3/?file=396978#file396978line145> > > > > 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) { > > ... > > } done. > On Dec. 12, 2013, 3:08 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java, line 68 > > <https://reviews.apache.org/r/16220/diff/3/?file=396978#file396978line68> > > > > 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. I don't think it is worth it here. If you disagree I can encapsulate all of the logic into a stateful private class that does not make multiple calls. - Zameer ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16220/#review30294 ----------------------------------------------------------- On Dec. 12, 2013, 12: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, 12: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 > >
