----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18312/#review35022 -----------------------------------------------------------
In HostResourceProvider it looks like you track a set of cluster names for hosts that are in implied passive state. So, even though we only support a single cluster now, this code can deal with multiple clusters. It looks like in ServiceResourceProvider you have similar code but always assign the cluster name to a single property, potentially overwriting the previous value. Again, since we only allow a single cluster now it shouldn't matter. Just curious why you handled it differently in the two providers? I think it better to account for the multiple cluster names now so that we don't have to deal with it down the line. - Tom Beerbower On Feb. 20, 2014, 4:23 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18312/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2014, 4:23 p.m.) > > > Review request for Ambari and Tom Beerbower. > > > Bugs: AMBARI-4761 > https://issues.apache.org/jira/browse/AMBARI-4761 > > > Repository: ambari > > > Description > ------- > > Small change to collect cluster names for maintenance state, then issue only > one command, instead of one-per-change. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java > d607eb4 > > ambari-server/src/main/java/org/apache/ambari/server/controller/PassiveStateHelper.java > 4d412f7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java > 01163b7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java > 9cbd16f > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > 2a1cf20 > > ambari-server/src/test/java/org/apache/ambari/server/controller/PassiveStateHelperTest.java > 8b4ad30 > > Diff: https://reviews.apache.org/r/18312/diff/ > > > Testing > ------- > > * Marked @Ignore on failing test in AmbariManagementControllerTest > * Added test scenarios for multiple hosts > > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 12:57.340s > [INFO] Finished at: Thu Feb 20 10:39:21 EST 2014 > [INFO] Final Memory: 18M/123M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Nate Cole > >
