ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dmitro Lisnichenko" <dlysniche...@hortonworks.com>
Subject Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack
Date Mon, 31 Aug 2015 09:12:21 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message