Re: Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Nate Cole]

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



This has been pushed.  Please close this review and jira.

- Nate Cole


On Jan. 3, 2018, 2:03 a.m., Yussuf Shaikh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64887/
> ---
> 
> (Updated Jan. 3, 2018, 2:03 a.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-22403
> https://issues.apache.org/jira/browse/AMBARI-22403
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Java side changes required to read JAVA HOME value for each host depending on 
> the OS type eg: java.home.redhat7.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  182cd58 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
>  c0a7a7b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  c4df0b1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  198b617 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  dcafdea 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
> b6287e6 
>   
> ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
>  e47fcd2 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
>  6bece66 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  e795d28 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  a1f28f1 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
>  c5994c5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
>  51035ba 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
>  670c187 
>   
> ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java
>  b90295d 
> 
> 
> Diff: https://reviews.apache.org/r/64887/diff/2/
> 
> 
> Testing
> ---
> 
> tests ran on ambari-server project.
> 
> 
> Thanks,
> 
> Yussuf Shaikh
> 
>

Re: Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Nate Cole]

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


Ship it!




Ship It!

- Nate Cole


On Jan. 3, 2018, 2:03 a.m., Yussuf Shaikh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64887/
> ---
> 
> (Updated Jan. 3, 2018, 2:03 a.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-22403
> https://issues.apache.org/jira/browse/AMBARI-22403
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Java side changes required to read JAVA HOME value for each host depending on 
> the OS type eg: java.home.redhat7.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  182cd58 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
>  c0a7a7b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  c4df0b1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  198b617 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  dcafdea 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
> b6287e6 
>   
> ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
>  e47fcd2 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
>  6bece66 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  e795d28 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  a1f28f1 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
>  c5994c5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
>  51035ba 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
>  670c187 
>   
> ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java
>  b90295d 
> 
> 
> Diff: https://reviews.apache.org/r/64887/diff/2/
> 
> 
> Testing
> ---
> 
> tests ran on ambari-server project.
> 
> 
> Thanks,
> 
> Yussuf Shaikh
> 
>

Re: Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Nate Cole]

> On Jan. 2, 2018, 11:51 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java
> > Line 633 (original)
> > 
> >
> > It's much more convenient to keep JAVA_HOME in one place.  We want to 
> > centralize (as much as we can) the places where we have to remember to set 
> > JAVA_HOME.  Can you pass in the os type or OS (the object0 here instead?
> 
> Yussuf Shaikh wrote:
> Here we do not know the os type of each host. This method is  called 
> initially during stage creation. Hence had introduced a method in 
> Configuration class which is called in hosts loop later for different flows.

Fair enough


- Nate


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


On Jan. 3, 2018, 2:03 a.m., Yussuf Shaikh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64887/
> ---
> 
> (Updated Jan. 3, 2018, 2:03 a.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-22403
> https://issues.apache.org/jira/browse/AMBARI-22403
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Java side changes required to read JAVA HOME value for each host depending on 
> the OS type eg: java.home.redhat7.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  182cd58 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
>  c0a7a7b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  c4df0b1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  198b617 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  dcafdea 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
> b6287e6 
>   
> ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
>  e47fcd2 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
>  6bece66 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  e795d28 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  a1f28f1 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
>  c5994c5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
>  51035ba 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
>  670c187 
>   
> ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java
>  b90295d 
> 
> 
> Diff: https://reviews.apache.org/r/64887/diff/2/
> 
> 
> Testing
> ---
> 
> tests ran on ambari-server project.
> 
> 
> Thanks,
> 
> Yussuf Shaikh
> 
>

Re: Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Yussuf Shaikh]

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

(Updated Jan. 3, 2018, 7:03 a.m.)


Review request for Ambari and Nate Cole.


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


Repository: ambari


Description
---

Java side changes required to read JAVA HOME value for each host depending on 
the OS type eg: java.home.redhat7.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 182cd58 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
 c0a7a7b 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
 c4df0b1 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 198b617 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
 dcafdea 
  ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
b6287e6 
  
ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
 e47fcd2 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
 6bece66 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
 e795d28 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
 a1f28f1 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
 c5994c5 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
 51035ba 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
 670c187 
  
ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java 
b90295d 


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

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


Testing
---

tests ran on ambari-server project.


Thanks,

Yussuf Shaikh

Re: Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Yussuf Shaikh]

> On Jan. 2, 2018, 4:51 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 3058-3065 (original), 3057-3060 (patched)
> > 
> >
> > The old logic here is still sound, it would help for when people add 
> > properties without having to restart Ambari to get them (not changed 
> > properties, but new ones which is fine).

Agreed. I will keep both functions here.


> On Jan. 2, 2018, 4:51 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java
> > Line 633 (original)
> > 
> >
> > It's much more convenient to keep JAVA_HOME in one place.  We want to 
> > centralize (as much as we can) the places where we have to remember to set 
> > JAVA_HOME.  Can you pass in the os type or OS (the object0 here instead?

Here we do not know the os type of each host. This method is  called initially 
during stage creation. Hence had introduced a method in Configuration class 
which is called in hosts loop later for different flows.


- Yussuf


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


On Jan. 2, 2018, 2:54 p.m., Yussuf Shaikh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64887/
> ---
> 
> (Updated Jan. 2, 2018, 2:54 p.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-22403
> https://issues.apache.org/jira/browse/AMBARI-22403
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Java side changes required to read JAVA HOME value for each host depending on 
> the OS type eg: java.home.redhat7.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  182cd58 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
>  c0a7a7b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  c4df0b1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  198b617 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  dcafdea 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
> b6287e6 
>   
> ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
>  e47fcd2 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
>  6bece66 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  e795d28 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  a1f28f1 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
>  c5994c5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
>  51035ba 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
>  670c187 
>   
> ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java
>  b90295d 
> 
> 
> Diff: https://reviews.apache.org/r/64887/diff/1/
> 
> 
> Testing
> ---
> 
> tests ran on ambari-server project.
> 
> 
> Thanks,
> 
> Yussuf Shaikh
> 
>

Re: Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Nate Cole]

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




ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 3058-3065 (original), 3057-3060 (patched)


The old logic here is still sound, it would help for when people add 
properties without having to restart Ambari to get them (not changed 
properties, but new ones which is fine).



ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java
Line 633 (original)


It's much more convenient to keep JAVA_HOME in one place.  We want to 
centralize (as much as we can) the places where we have to remember to set 
JAVA_HOME.  Can you pass in the os type or OS (the object0 here instead?


- Nate Cole


On Jan. 2, 2018, 9:54 a.m., Yussuf Shaikh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64887/
> ---
> 
> (Updated Jan. 2, 2018, 9:54 a.m.)
> 
> 
> Review request for Ambari and Nate Cole.
> 
> 
> Bugs: AMBARI-22403
> https://issues.apache.org/jira/browse/AMBARI-22403
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Java side changes required to read JAVA HOME value for each host depending on 
> the OS type eg: java.home.redhat7.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  182cd58 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
>  c0a7a7b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  c4df0b1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  198b617 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  dcafdea 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
> b6287e6 
>   
> ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
>  e47fcd2 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
>  6bece66 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  e795d28 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  a1f28f1 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
>  c5994c5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
>  51035ba 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
>  670c187 
>   
> ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java
>  b90295d 
> 
> 
> Diff: https://reviews.apache.org/r/64887/diff/1/
> 
> 
> Testing
> ---
> 
> tests ran on ambari-server project.
> 
> 
> Thanks,
> 
> Yussuf Shaikh
> 
>

Review Request 64887: AMBARI-22403 Read the JAVA_HOME depending on the OS family during Service install (action & command) and upgrade

[Yussuf Shaikh]

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

Review request for Ambari and Nate Cole.


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


Repository: ambari


Description
---

Java side changes required to read JAVA HOME value for each host depending on 
the OS type eg: java.home.redhat7.


Diffs
-

  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 182cd58 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java
 c0a7a7b 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
 c4df0b1 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 198b617 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
 dcafdea 
  ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java 
b6287e6 
  
ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
 e47fcd2 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java
 6bece66 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
 e795d28 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
 a1f28f1 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
 c5994c5 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
 51035ba 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProviderTest.java
 670c187 
  
ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java 
b90295d 


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


Testing
---

tests ran on ambari-server project.


Thanks,

Yussuf Shaikh