----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4732/#review6961 -----------------------------------------------------------
/trunk/src/java/org/apache/hcatalog/common/HCatLogUtil.java <https://reviews.apache.org/r/4732/#comment15507> You can pass the loglevel as argument to the method and cut down on the number of methods. slf4j does not have support for it like other logging systems, so you will have to do a switch case statement. http://stackoverflow.com/questions/2621701/setting-log-level-of-message-at-runtime-in-slf4j /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java <https://reviews.apache.org/r/4732/#comment15508> Nitpick. It would be easier for readability and use if the static logger variable name is all caps. /trunk/src/java/org/apache/hcatalog/data/JsonSerDe.java <https://reviews.apache.org/r/4732/#comment15509> We can remove e.getMessage() if we are logging t. If we only want to log the message, then t is not required. /trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java <https://reviews.apache.org/r/4732/#comment15510> {} is not required here. Couple of other places too - Rohini 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 > >
