----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28472/#review63110 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105300> No references to IO should be in this class ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105299> Belongs in StackDirectory ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105296> This logic should occur prior to mergeServicesWithExplicitParent() invocaiton. Get rid of this if block and use the existing one above. ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105293> StackModule shouldn't do any IO operations. IO is abstracted via StackDirectory. ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105294> The RCO file location is available from StackDirectory. Should update StackDirectory to to all processing of this file. ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105302> Since the logic is fairly substantial to merge RCO data, it should probably represented in a separate class, something lile StackRoleCommandOrder. StackDirectory would parse the file into an instance of this class and StackModule would get it from StackDirectory. To merge you could just call a merge method, perhaps something like 'StackRoleCommandOrder merge(StackRoleCommandOrder rco)'. ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105297> All IO belongs in StackDirectory. ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java <https://reviews.apache.org/r/28472/#comment105298> This method belongs in StackDirectory. Introduce a parse and get method in StackDirectory for RCO data. - John Speidel On Nov. 26, 2014, 2:50 p.m., Andrew Onischuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28472/ > ----------------------------------------------------------- > > (Updated Nov. 26, 2014, 2:50 p.m.) > > > Review request for Ambari and Dmitro Lisnichenko. > > > Bugs: AMBARI-4504 > https://issues.apache.org/jira/browse/AMBARI-4504 > > > Repository: ambari > > > Description > ------- > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/metadata/RoleCommandOrder.java > 32668fa > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java > 20dba84 > ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java > f19cf81 > ambari-server/src/main/resources/stacks/HDP/2.1/role_command_order.json > 2d152ba > ambari-server/src/main/resources/stacks/HDP/2.2/role_command_order.json > 72b49fa > > ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerTest.java > 0502e1a > ambari-server/src/test/resources/stacks/HDP/2.0.8/role_command_order.json > 1404ef6 > ambari-server/src/test/resources/stacks/HDP/2.1.1/role_command_order.json > 1404ef6 > > Diff: https://reviews.apache.org/r/28472/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Andrew Onischuk > >
