You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Attila Doroszlai <ad...@hortonworks.com> on 2017/07/03 10:32:00 UTC

Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

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

Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.


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


Repository: ambari


Description
-------

Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
  ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
  ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
  ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 


Diff: https://reviews.apache.org/r/60595/diff/1/


Testing
-------

Manual test on local cluster.
New unit test.


Thanks,

Attila Doroszlai


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

Posted by Attila Doroszlai <ad...@hortonworks.com>.

> On July 3, 2017, 4:09 p.m., Jonathan Hurley wrote:
> > I don't know why it's not showing Fix It, Then Ship It - but that's what I picked. Either way, I'll Ship It directly and you can correct the above comments.

With the new design Review Board shows "Fix It" on top of "Ship It" (the green background is slightly visible) instead of "Fix It, then Ship It".


- Attila


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


On July 3, 2017, 4:19 p.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60595/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 4:19 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-21390
>     https://issues.apache.org/jira/browse/AMBARI-21390
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
> The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60595/diff/2/
> 
> 
> Testing
> -------
> 
> Manual test on local cluster.
> New unit test.
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On July 3, 2017, 10:09 a.m., Jonathan Hurley wrote:
> > I don't know why it's not showing Fix It, Then Ship It - but that's what I picked. Either way, I'll Ship It directly and you can correct the above comments.
> 
> Attila Doroszlai wrote:
>     With the new design Review Board shows "Fix It" on top of "Ship It" (the green background is slightly visible) instead of "Fix It, then Ship It".

Not the most intuitive :)


- Jonathan


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


On July 3, 2017, 10:19 a.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60595/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 10:19 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-21390
>     https://issues.apache.org/jira/browse/AMBARI-21390
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
> The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60595/diff/2/
> 
> 
> Testing
> -------
> 
> Manual test on local cluster.
> New unit test.
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60595/#review179505
-----------------------------------------------------------


Ship it!




I don't know why it's not showing Fix It, Then Ship It - but that's what I picked. Either way, I'll Ship It directly and you can correct the above comments.

- Jonathan Hurley


On July 3, 2017, 6:32 a.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60595/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 6:32 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-21390
>     https://issues.apache.org/jira/browse/AMBARI-21390
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
> The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60595/diff/1/
> 
> 
> Testing
> -------
> 
> Manual test on local cluster.
> New unit test.
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

Posted by Alejandro Fernandez <af...@hortonworks.com>.

> On July 5, 2017, 5:09 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/60595/diff/2/?file=1768456#file1768456line74>
> >
> >     We need to change other things just besides the stack root.
> >     There are other references to 
> >     
> >     iop/apps
> >     iop-select
> >     
> >     Because this is specific to IOP to HDP, I suggest putting IOPtoHDP in the class name.

Also things like $iop.version


- Alejandro


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


On July 3, 2017, 2:19 p.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60595/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 2:19 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-21390
>     https://issues.apache.org/jira/browse/AMBARI-21390
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
> The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60595/diff/2/
> 
> 
> Testing
> -------
> 
> Manual test on local cluster.
> New unit test.
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

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




ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java
Lines 74 (patched)
<https://reviews.apache.org/r/60595/#comment254463>

    We need to change other things just besides the stack root.
    There are other references to 
    
    iop/apps
    iop-select
    
    Because this is specific to IOP to HDP, I suggest putting IOPtoHDP in the class name.


- Alejandro Fernandez


On July 3, 2017, 2:19 p.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60595/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 2:19 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-21390
>     https://issues.apache.org/jira/browse/AMBARI-21390
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
> The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60595/diff/2/
> 
> 
> Testing
> -------
> 
> Manual test on local cluster.
> New unit test.
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

Posted by Attila Doroszlai <ad...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60595/
-----------------------------------------------------------

(Updated July 3, 2017, 4:19 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.


Changes
-------

addressed review comments


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


Repository: ambari


Description
-------

Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
  ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
  ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
  ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 


Diff: https://reviews.apache.org/r/60595/diff/2/

Changes: https://reviews.apache.org/r/60595/diff/1-2/


Testing
-------

Manual test on local cluster.
New unit test.


Thanks,

Attila Doroszlai


Re: Review Request 60595: AMBARI-21390. Change stack root references in config during cross-stack upgrade

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60595/#review179502
-----------------------------------------------------------


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java
Lines 40 (patched)
<https://reviews.apache.org/r/60595/#comment254246>

    That would be an awesome new feature!



ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java
Lines 60-61 (patched)
<https://reviews.apache.org/r/60595/#comment254247>

    Clusters.getCluster(String) throws a ClusterNotFoundException - no need for the NULL check.



ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java
Lines 68-69 (patched)
<https://reviews.apache.org/r/60595/#comment254248>

    Not sure how this could be null - probably don't need this check.



ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java
Lines 72 (patched)
<https://reviews.apache.org/r/60595/#comment254249>

    Use System.lineSeparator here instead of \n


- Jonathan Hurley


On July 3, 2017, 6:32 a.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60595/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 6:32 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-21390
>     https://issues.apache.org/jira/browse/AMBARI-21390
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Old and new stack root paths are hard-coded for now.  I will try to make it configurable later.
> The patch also copies BigInsights 4.2 upgrade pack to 4.2.5 to be able to upgrade from the latter version.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryAction.java PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2.5/upgrades/nonrolling-upgrade-to-hdp-2.6.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/BigInsights/4.2/upgrades/nonrolling-upgrade-to-hdp-2.6.xml e8a1e33545c17faaf5856b77b77c4621249bb4ac 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ChangeStackRootDirectoryActionTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60595/diff/1/
> 
> 
> Testing
> -------
> 
> Manual test on local cluster.
> New unit test.
> 
> 
> Thanks,
> 
> Attila Doroszlai
> 
>