> On 2012-04-16 22:54:11, Rohini Palaniswamy wrote:
> > /trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java,
> >  line 104
> > <https://reviews.apache.org/r/4732/diff/2/?file=102233#file102233line104>
> >
> >     {} is not required here. Couple of other places too
> 
> Vandana Ayyalasomayajula wrote:
>     if the {} is not used then the exception will not be logged. So either we 
> do:
>     
>     if( logger.isWarnEnabled()){
>     logger.warn("Failed to cancel delegation token," + e);
>     }
>     
>     or   logger.warn("Failed to cancel delegation token, {}", e);
>     
>
> 
> Rohini Palaniswamy wrote:
>      /**
>        * Log an exception (throwable) at the WARN level with an
>        * accompanying message. 
>        * 
>        * @param msg the message accompanying the exception
>        * @param t the exception (throwable) to log 
>        */
>       public void warn(String msg, Throwable t);
>     
>      is the commonly used API and should work without doing {} or +.

what you said is right. Now the code snipped will look like. 

if(log.isWarnEnabled()){

log.warn(comment, e);

}

I was trying to take advantage of the parametrized messages offered by the 
SLF4J. It does not use String.format in its internal implementation.


- Vandana


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


On 2012-04-16 21:16:58, Vandana Ayyalasomayajula wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4732/
> -----------------------------------------------------------
> 
> (Updated 2012-04-16 21:16:58)
> 
> 
> Review request for hcatalog, Sushanth Sowmyan, Francis Liu, and Thomas.
> 
> 
> Summary
> -------
> 
> We need to do the following things:
> 
> + Use log4j across the board
> + Introduce a class with methods to help logging, refactoring away the bunch 
> of these functions in HCatUtil right now
> + Instrument debug logging with isLogEnabled checks on the log level
> + remove all instances of commented code whose purpose was for debug logs
> 
> 
> This addresses bug hcatalog-68.
>     https://issues.apache.org/jira/browse/hcatalog-68
> 
> 
> Diffs
> -----
> 
>   
> /trunk/src/java/org/apache/hcatalog/cli/SemanticAnalysis/HCatSemanticAnalyzer.java
>  1325838 
>   /trunk/src/java/org/apache/hcatalog/common/HCatLogUtil.java PRE-CREATION 
>   /trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1325838 
>   /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java 
> 1325838 
>   
> /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java
>  1325838 
>   /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java 1325838 
>   /trunk/src/java/org/apache/hcatalog/data/JsonSerDe.java 1325838 
>   
> /trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java
>  1325838 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1325838 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1325838 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1325838 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1325838 
>   /trunk/src/test/org/apache/hcatalog/data/HCatDataCheckUtil.java 1325838 
>   /trunk/src/test/org/apache/hcatalog/data/TestJsonSerDe.java 1325838 
> 
> Diff: https://reviews.apache.org/r/4732/diff
> 
> 
> Testing
> -------
> 
> Unit tests pass. Log messages verified from test logs.
> 
> 
> Thanks,
> 
> Vandana
> 
>

Reply via email to