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



Does logging need to change in any of other classes of the tez view too?


contrib/views/tez/src/main/java/org/apache/ambari/view/tez/ViewControllerImpl.java
 (line 77)
<https://reviews.apache.org/r/48686/#comment202657>

    Fix typo: fetching. 
    
    Error message is not clear. The current message seems to be indicate that 
we failed to fetch data from a particular URL however the function is trying to 
find the location of the timeline server. 
    
    Maybe change to "Failed to find Active ResourceManager location" or "Failed 
to find YARN Timeline Server location". 
    
    Please do not use ATS as users are familiar with YARN Timeline Server and 
not ATS.



contrib/views/tez/src/main/java/org/apache/ambari/view/tez/ViewControllerImpl.java
 (line 87)
<https://reviews.apache.org/r/48686/#comment202658>

    See above comment on typo and message



contrib/views/tez/src/main/java/org/apache/ambari/view/tez/ViewControllerImpl.java
 (line 97)
<https://reviews.apache.org/r/48686/#comment202659>

    See above comment on typo and message
    
    Here, something like "Failed to find YARN http/https protocol configuration 
value"



contrib/views/tez/src/main/java/org/apache/ambari/view/tez/exceptions/YARNProtocolFetchException.java
 (line 28)
<https://reviews.apache.org/r/48686/#comment202660>

    WebApplicationException(String message, Throwable cause, int status)
    or 
    WebApplicationException(String message, Throwable cause)
    
    Any reason we are not using the above ctor for the super(). We can then 
inject a custom message here which will be displayed in the stack trace 
returned.  This could be done for all the new exception classes in the tez view 
codebase.



contrib/views/tez/src/main/java/org/apache/ambari/view/tez/rest/BaseProxyResource.java
 (line 66)
<https://reviews.apache.org/r/48686/#comment202661>

    At debug level, we should log the response too. 
    
    i.e. 
    
    if LOG.isDebugEnabled() {
       LOG.error( ... failed to parse, url, response )
    }


- Hitesh Shah


On June 14, 2016, 12:28 p.m., Sreenath Somarajapuram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48686/
> -----------------------------------------------------------
> 
> (Updated June 14, 2016, 12:28 p.m.)
> 
> 
> Review request for Ambari, DIPAYAN BHOWMICK and Pallav Kulshreshtha.
> 
> 
> Bugs: AMBARI-17186
>     https://issues.apache.org/jira/browse/AMBARI-17186
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> More log messages would help us to debug EARs faster.
> 
> 
> Diffs
> -----
> 
>   
> contrib/views/tez/src/main/java/org/apache/ambari/view/tez/ViewControllerImpl.java
>  981353b 
>   
> contrib/views/tez/src/main/java/org/apache/ambari/view/tez/exceptions/YARNProtocolFetchException.java
>  PRE-CREATION 
>   
> contrib/views/tez/src/main/java/org/apache/ambari/view/tez/rest/BaseProxyResource.java
>  619f81e 
>   
> contrib/views/tez/src/main/java/org/apache/ambari/view/tez/rest/BaseRedirectionResource.java
>  97ee01d 
> 
> Diff: https://reviews.apache.org/r/48686/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> Verified the presence of debug and error logs in 
> /var/log/ambari-server/tez-view/tez-view.log
> 
> 
> Thanks,
> 
> Sreenath Somarajapuram
> 
>

Reply via email to