You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Di Li <di...@ca.ibm.com> on 2016/02/24 14:54:14 UTC

Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/
-----------------------------------------------------------

Review request for Ambari and Alejandro Fernandez.


Bugs: AMBARI-15132
    https://issues.apache.org/jira/browse/AMBARI-15132


Repository: ambari


Description
-------

<condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 

Diff: https://reviews.apache.org/r/43944/diff/


Testing
-------

unit test
ran upgrade from Ambari 2.2 to Ambari trunk.


Thanks,

Di Li


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120554
-----------------------------------------------------------


Ship it!




Ship It!

- Alejandro Fernandez


On Feb. 24, 2016, 1:54 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 1:54 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.

> On Feb. 24, 2016, 7:35 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml, line 129
> > <https://reviews.apache.org/r/43944/diff/1/?file=1267519#file1267519line129>
> >
> >     Do we already have granularity at the individual op level?
> >     E.g.,
> >     <transfer ... if-key="" if-value="" />
> >     <replace ... if-key="" if-value="" />
> >     <set ... if-key="" if-value="" />
> 
> Di Li wrote:
>     Hello Alejandro,
>     
>     Ambari trunk code currently only have conditions for transfter operation "delete".
>     
>     My initial thought was to do all the condition in the  <condition> section. Based on Nate and your comments , I am updating the patch to have conditions done at operation level just like the ones you listed.

Hello Alejandro,

Could you please reveiw the new patch? It has the conditions in  set/transfer/replace.


- Di


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120556
-----------------------------------------------------------


On Feb. 24, 2016, 1:54 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 1:54 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.

> On Feb. 24, 2016, 7:35 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml, line 129
> > <https://reviews.apache.org/r/43944/diff/1/?file=1267519#file1267519line129>
> >
> >     Do we already have granularity at the individual op level?
> >     E.g.,
> >     <transfer ... if-key="" if-value="" />
> >     <replace ... if-key="" if-value="" />
> >     <set ... if-key="" if-value="" />

Hello Alejandro,

Ambari trunk code currently only have conditions for transfter operation "delete".

My initial thought was to do all the condition in the  <condition> section. Based on Nate and your comments , I am updating the patch to have conditions done at operation level just like the ones you listed.


- Di


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120556
-----------------------------------------------------------


On Feb. 24, 2016, 1:54 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 1:54 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120556
-----------------------------------------------------------




ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml (line 128)
<https://reviews.apache.org/r/43944/#comment182022>

    Do we already have granularity at the individual op level?
    E.g.,
    <transfer ... if-key="" if-value="" />
    <replace ... if-key="" if-value="" />
    <set ... if-key="" if-value="" />


- Alejandro Fernandez


On Feb. 24, 2016, 1:54 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 1:54 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.

> On Feb. 24, 2016, 3:48 p.m., Nate Cole wrote:
> > It seems like we should be going the other way.  set/replace should support conditional attributes.  The <transfer> already supports this (see HDP/2.3/config-upgrade.xml for dfs.namenode.rpc-address).  <condition> should not be used

Got it Nate. I will update the patch accordingly.


- Di


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120509
-----------------------------------------------------------


On Feb. 24, 2016, 1:54 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 1:54 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120509
-----------------------------------------------------------



It seems like we should be going the other way.  set/replace should support conditional attributes.  The <transfer> already supports this (see HDP/2.3/config-upgrade.xml for dfs.namenode.rpc-address).  <condition> should not be used

- Nate Cole


On Feb. 24, 2016, 8:54 a.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 8:54 a.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 4ea8f15 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.

> On Feb. 25, 2016, 7:49 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java, lines 445-460
> > <https://reviews.apache.org/r/43944/diff/2/?file=1268445#file1268445line445>
> >
> >     These properties can all be abstracted to Masked - Transfer and ConfigurationKeyValue ("set") already extend that.
> >     
> >     Also, probably want to remove <condition> from XML.

Hello Nate,

I updated the code to mve the if-key, etc to the Masked. Please review.

Thanks.


- Di


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120749
-----------------------------------------------------------


On Feb. 24, 2016, 10:32 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 10:32 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120749
-----------------------------------------------------------




ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java (lines 445 - 460)
<https://reviews.apache.org/r/43944/#comment182218>

    These properties can all be abstracted to Masked - Transfer and ConfigurationKeyValue ("set") already extend that.
    
    Also, probably want to remove <condition> from XML.


- Nate Cole


On Feb. 24, 2016, 5:32 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 5:32 p.m.)
> 
> 
> Review request for Ambari and Alejandro Fernandez.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.

> On Feb. 26, 2016, 2:27 a.m., Nate Cole wrote:
> > This is very close - I think there's just one more thing to do (below).

Hello Nate,

Please review the lastest patch where the ConfigureTask only does basic if attribute validation like you suggested (like has if-value but no if-key). and the ConfigureAction.java to perform the allow/disallow check.


- Di


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120815
-----------------------------------------------------------


On Feb. 25, 2016, 9:34 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2016, 9:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120815
-----------------------------------------------------------



This is very close - I think there's just one more thing to do (below).


ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java (lines 251 - 252)
<https://reviews.apache.org/r/43944/#comment182315>

    This shouldn't be done here.  ConfigureAction handles largely handles the transfer/set/replacement "property existence" (but will have to be modified for this patch).  The "get allowed" should only be checking for invalid attributes like "if-value is present, but if-key is not" type of stuff.  (If we had an XSD that would work better, but that's a much different JIRA :) )
    
    Also, when going across stacks there may be directives against properties that ONLY come with the new stack but may not be there when this code is invoked (orchestration).



ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java (lines 274 - 277)
<https://reviews.apache.org/r/43944/#comment182316>

    See above


- Nate Cole


On Feb. 25, 2016, 4:34 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2016, 4:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review121089
-----------------------------------------------------------


Ship it!




Ship It!

- Nate Cole


On Feb. 26, 2016, 5:28 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 5:28 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java e77cf36 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java ce87668 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 10:28 p.m.)


Review request for Ambari, Alejandro Fernandez and Nate Cole.


Bugs: AMBARI-15132
    https://issues.apache.org/jira/browse/AMBARI-15132


Repository: ambari


Description
-------

<condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java e77cf36 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java ce87668 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 

Diff: https://reviews.apache.org/r/43944/diff/


Testing
-------

unit test
ran upgrade from Ambari 2.2 to Ambari trunk.


Thanks,

Di Li


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.

> On Feb. 26, 2016, 10:13 p.m., Nate Cole wrote:
> > Nice work!  Just remove dead comments and I'm all set on this review.  Thanks for taking this up, it really helps the consistency and readability of the upgrade packs.

Hello Nate,

Sorry about the dead comments. I was meant to remove them in the previous patch submit.

Please review the latest patch.

Thanks


- Di


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120978
-----------------------------------------------------------


On Feb. 26, 2016, 9:47 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 9:47 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java e77cf36 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java ce87668 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/#review120978
-----------------------------------------------------------


Fix it, then Ship it!




Nice work!  Just remove dead comments and I'm all set on this review.  Thanks for taking this up, it really helps the consistency and readability of the upgrade packs.


ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java (lines 325 - 329)
<https://reviews.apache.org/r/43944/#comment182531>

    remove commented dead-code


- Nate Cole


On Feb. 26, 2016, 4:47 p.m., Di Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43944/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 4:47 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-15132
>     https://issues.apache.org/jira/browse/AMBARI-15132
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> <condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java e77cf36 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java ce87668 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 
> 
> Diff: https://reviews.apache.org/r/43944/diff/
> 
> 
> Testing
> -------
> 
> unit test
> ran upgrade from Ambari 2.2 to Ambari trunk.
> 
> 
> Thanks,
> 
> Di Li
> 
>


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 9:47 p.m.)


Review request for Ambari, Alejandro Fernandez and Nate Cole.


Bugs: AMBARI-15132
    https://issues.apache.org/jira/browse/AMBARI-15132


Repository: ambari


Description
-------

<condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java e77cf36 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java ce87668 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 

Diff: https://reviews.apache.org/r/43944/diff/


Testing
-------

unit test
ran upgrade from Ambari 2.2 to Ambari trunk.


Thanks,

Di Li


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/
-----------------------------------------------------------

(Updated Feb. 25, 2016, 9:34 p.m.)


Review request for Ambari, Alejandro Fernandez and Nate Cole.


Bugs: AMBARI-15132
    https://issues.apache.org/jira/browse/AMBARI-15132


Repository: ambari


Description
-------

<condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 

Diff: https://reviews.apache.org/r/43944/diff/


Testing
-------

unit test
ran upgrade from Ambari 2.2 to Ambari trunk.


Thanks,

Di Li


Re: Review Request 43944: AMBARI-15132 in upgrade.xml to support set/replace/transfer

Posted by Di Li <di...@ca.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43944/
-----------------------------------------------------------

(Updated Feb. 24, 2016, 10:32 p.m.)


Review request for Ambari and Alejandro Fernandez.


Bugs: AMBARI-15132
    https://issues.apache.org/jira/browse/AMBARI-15132


Repository: ambari


Description
-------

<condition> in upgrade.xml currently only supports single property set. this is to improve it to to support set/replace/transfer. The code change must be backward compatible in order to not break the existing update-**.xml and nonrolling-upgrade-**.xml.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigUpgradeChangeDefinition.java a4dff2e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 6b22f58 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 1a5d4e7 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/config-upgrade.xml f345d12 
  ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml 237da77 

Diff: https://reviews.apache.org/r/43944/diff/


Testing
-------

unit test
ran upgrade from Ambari 2.2 to Ambari trunk.


Thanks,

Di Li