> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, > > line 49 > > <https://reviews.apache.org/r/37682/diff/5/?file=1056027#file1056027line49> > > > > Can you switch this to fully qualified imports?
ok > On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java, > > line 429 > > <https://reviews.apache.org/r/37682/diff/5/?file=1056029#file1056029line429> > > > > I'm ok with this convention. Can we make the comparison case > > insensitive? good catch > On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, > > line 207 > > <https://reviews.apache.org/r/37682/diff/5/?file=1056032#file1056032line207> > > > > Does initializing the ArrayList require passing in the type as > > <UpgradeGroupHolder>? Since Java 1.7, Diamond operator allows compiler to infer generic type > On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, > > line 235 > > <https://reviews.apache.org/r/37682/diff/5/?file=1056032#file1056032line235> > > > > Same comment about type in the ArrayList here. the same > On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java, > > line 151 > > <https://reviews.apache.org/r/37682/diff/5/?file=1056034#file1056034line151> > > > > Why did the XmlElement names have to be removed? Since we move all configuration changes to a separate file (`upgrade-config-changes-*.xml`), configure task definition is not going to be supported anymore in upgrade pack. We generate relevant configure tasks in runtime. Moreover, in responce to use cases mentioned by Jonathan, I'm going to add per-service config change priority. > On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote: > > ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml, > > line 19 > > <https://reviews.apache.org/r/37682/diff/5/?file=1056040#file1056040line19> > > > > Great, this is the same structure I had in mind. > > We'll need to be careful when merging this to branch-2.1 and trunk so > > that we don't forget any properties. > > For now, we only need to verify it works for core services (HDFS, YARN, > > MR, HBASE, ZK). good point - Dmitro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37682/#review96699 ----------------------------------------------------------- On Aug. 27, 2015, 3:08 p.m., Dmitro Lisnichenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37682/ > ----------------------------------------------------------- > > (Updated Aug. 27, 2015, 3:08 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan > Hurley, and Nate Cole. > > > Bugs: AMBARI-12700 > https://issues.apache.org/jira/browse/AMBARI-12700 > > > Repository: ambari > > > Description > ------- > > The configs need to move out of the Upgrade Packs and into their own file. > This will make it easier to maintain, and clearer since there will not be any > dups. > > Since it is going to be a massive change, it would be great to get early > feedback. Code is not complete (still full of TODOs and does not even build) > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > 4afa9b0 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > fa743be > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java > c717582 > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java > 8f81b5a > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java > 89c10c6 > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java > 4b88aff > ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java > 2aa89cc > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java > 3e25d01 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java > 8361ea6 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java > 8361ea6 > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml > 9b7848f > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java > 93e29b5 > > ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java > f7898ee > > ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java > a73775f > > ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml > PRE-CREATION > > Diff: https://reviews.apache.org/r/37682/diff/ > > > Testing > ------- > > just published preview of changes > > > Thanks, > > Dmitro Lisnichenko > >
