Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Jonathan Hurley 

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


Ship it!




Ship It!

- Jonathan Hurley


On June 28, 2017, 11:16 a.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 28, 2017, 11:16 a.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/3/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Attila Doroszlai 

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


Ship it!




Ship It!

- Attila Doroszlai


On June 28, 2017, 5:16 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 28, 2017, 5:16 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/3/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Di Li 


> On June 28, 2017, 2:45 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
> > Lines 57 (patched)
> > 
> >
> > Probably not necessary...

you are right, I removed it.


> On June 28, 2017, 2:45 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
> > Lines 64 (patched)
> > 
> >
> > Is there an extra S on the end of this?

yeah, removed it.


- Di


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


On June 28, 2017, 3:16 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 28, 2017, 3:16 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/3/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Tim Thorpe 

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


Ship it!




Ship It!

- Tim Thorpe


On June 28, 2017, 3:16 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 28, 2017, 3:16 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/3/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Di Li 

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

(Updated June 28, 2017, 3:16 p.m.)


Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.


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


Repository: ambari


Description
---

This is for the JavaVersionCheck check defined in the cross stack upgrade pack 
EU xml file.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
 71c5857 
  
ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/60481/diff/3/

Changes: https://reviews.apache.org/r/60481/diff/2-3/


Testing
---

to be tested later on with the integrated tests.


Thanks,

Di Li

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Di Li 

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

(Updated June 28, 2017, 1:34 p.m.)


Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.


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


Repository: ambari


Description
---

This is for the JavaVersionCheck check defined in the cross stack upgrade pack 
EU xml file.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
 71c5857 
  
ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
 PRE-CREATION 


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

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


Testing
---

to be tested later on with the integrated tests.


Thanks,

Di Li

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Attila Doroszlai 

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




ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 100-101 (patched)


`Configuration.getJavaVersion` does not require any information from the 
`Configuration` instance, it only checks a system property.  Can you please 
make it `static` to avoid unnecessarily reading the config file?



ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 105 (patched)


This method duplicates most of `Configuration.getJavaVersion`.  This 
duplication could be reduced by making this method `public static`, and reusing 
it from `Configuration`.  Ideally, put it in some utility class.


- Attila Doroszlai


On June 27, 2017, 11:02 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 27, 2017, 11:02 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/1/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-28 Thread Attila Doroszlai 

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




ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
Lines 336 (patched)


Why put a specific version in the message here?



ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 34-35 (patched)


"Hive Metastore"?



ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 44 (patched)


Comment belongs to previous line, doesn't it?



ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 82 (patched)


Seems to have 2 arguments for single `%s`.

What does the message turn out in this case?  Isn't it something like "... 
requires at least JDK Java ..."?



ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 93 (patched)


`javaVersionS` may be null, which shouldn't be displayed in the formatted 
message



ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
Lines 95 (patched)


unnecessary


- Attila Doroszlai


On June 27, 2017, 11:02 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 27, 2017, 11:02 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/1/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-27 Thread Oliver Szabo 

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



just to notice this change (can be related for this): 
https://reviews.apache.org/r/59954/

- Oliver Szabo


On June 27, 2017, 9:02 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 27, 2017, 9:02 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/1/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-27 Thread Tim Thorpe 

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


Ship it!




Ship It!

- Tim Thorpe


On June 27, 2017, 9:02 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 27, 2017, 9:02 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/1/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>

Re: Review Request 60481: AMBARI-21359 Add JavaVersionCheck check

2017-06-27 Thread Alejandro Fernandez 

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


Ship it!




Ship It!

- Alejandro Fernandez


On June 27, 2017, 9:02 p.m., Di Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60481/
> ---
> 
> (Updated June 27, 2017, 9:02 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Sumit Mohanty, Sid Wagle, and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21359
> https://issues.apache.org/jira/browse/AMBARI-21359
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This is for the JavaVersionCheck check defined in the cross stack upgrade 
> pack EU xml file.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java
>  71c5857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/JavaVersionCheck.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60481/diff/1/
> 
> 
> Testing
> ---
> 
> to be tested later on with the integrated tests.
> 
> 
> Thanks,
> 
> Di Li
> 
>