> On Sept. 9, 2015, 10:02 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java, > > line 41 > > <https://reviews.apache.org/r/37682/diff/6/?file=1065309#file1065309line41> > > > > Can we change this to not include the term "patch". It will get > > confusing with the patch upgrade work coming soon. Maybe call it something > > like ConfigPack or ConfigurationPackage, etc. > > Dmitro Lisnichenko wrote: > I was trying to avoid confusion with config versions. How about > ConfigUpgradePackage?
I'm good with that. I just want to avoid the term "patch" in this feature. > On Sept. 9, 2015, 10:02 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, > > line 347 > > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line347> > > > > Any reason you are scoping these under a "changes" element? Seems > > redundant and unnecessary. > > Dmitro Lisnichenko wrote: > I thought we may add few other sections later, so wanted to have a > separate section for changes. > > Dmitro Lisnichenko wrote: > It would be great to get the patch in asap to unblock other work. If we > decide to remove <changes> section, may I address removing that in a next > patch (adding config changes to SWU)? OK. I'm fine with that. I just don't want to make this file overly verbose if it doesn't need to be. > On Sept. 9, 2015, 10:02 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, > > lines 345-346 > > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line345> > > > > Why are we placing the service/component in this configuration file? > > Seems like it's not needed. The IDs are referenced directly from the > > upgrade XML pack. Wouldn't this make it harder to lookup the IDs during > > upgrade? > > Dmitro Lisnichenko wrote: > That is not required for ids to work, but it makes file much more obvious > and manageable. So changes are grouped by owner and can be collapsed in IDE. > IDs are added to map and cached, and we don't deal with > services/components in runtime when performing upgrade I can see the value in that. But at the same time, it adds complexity to this file without any functional benefit (unless you can see a use case for querying for all configuration definitions by service/component). To unblock, I'm fine leaving it for now. Please open a Jira to revisit this and the below <changes> element to see if they should be removed. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37682/#review98176 ----------------------------------------------------------- On Sept. 9, 2015, 9:58 a.m., Dmitro Lisnichenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37682/ > ----------------------------------------------------------- > > (Updated Sept. 9, 2015, 9:58 a.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 > dddec73 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java > c717582 > > ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java > aa8e17b > > 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 > db947ca > 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 > 87301e5 > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java > ecefe6e > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.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/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java > 9d89b7a > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java > c9c6b8c > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml > PRE-CREATION > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml > 74eb499 > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml > 7c1a1f9 > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml > 7c1a1f9 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml > 044c43a > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java > e702e0a > > ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java > 2eee2df > > ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java > fc731d9 > > Diff: https://reviews.apache.org/r/37682/diff/ > > > Testing > ------- > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Ambari Views ...................................... SUCCESS [2.992s] > [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s] > [INFO] Ambari Server ..................................... SUCCESS > [54:25.287s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 54:30.573s > [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015 > [INFO] Final Memory: 60M/1072M > > > Thanks, > > Dmitro Lisnichenko > >
