> On Feb. 20, 2014, 1:20 p.m., Tom Beerbower wrote: > > 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. > > Nate Cole wrote: > Great question. The code in ServiceResourceProvider and AMC (for > HostComponents) each have a block (around line 1482 in the diff) that is > already throwing an Exception when trying to update more than one cluster. I > agree with you - I'll modify the code to just use clusternames set already > defined.
Correction - to use a set of clusternames, not the one already defined. Not all clusters may require the update. - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18312/#review35022 ----------------------------------------------------------- On Feb. 20, 2014, 11:23 a.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18312/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2014, 11:23 a.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 > >
