> On 2012-04-27 21:07:48, David Capwell wrote:
> > /trunk/src/java/org/apache/hcatalog/data/JsonSerDe.java, lines 153-156
> > <https://reviews.apache.org/r/4732/diff/9-10/?file=104702#file104702line153>
> >
> >     Why was this revered back?  patch 9 was using the throwable interface, 
> > this one reverts back to the old one using Object, Object
> 
> Vandana Ayyalasomayajula wrote:
>     The original source had the text causing the exception also logged. The 
> previous patch had only the exception being logged. Sushant pointed out that 
> this was resulting in reduced info on the log. I think this is okay as the 
> exception is again thrown.

So Vandana and I looked and this should be very descriptive saying the 
line/column number for where json failed and the json string itself.  This 
should be enough information from the user's point of view.


> On 2012-04-27 21:07:48, David Capwell wrote:
> > /trunk/src/test/org/apache/hcatalog/data/HCatDataCheckUtil.java, lines 84-86
> > <https://reviews.apache.org/r/4732/diff/9-10/?file=104707#file104707line84>
> >
> >     It might be best to wrap this around an ifInfo check since its going to 
> > loop.  I would assume info would be enabled more often then not so might 
> > not be needed.
> 
> Vandana Ayyalasomayajula wrote:
>     I think we don't need the loop at all taking a closer look. Its a list of 
> string.

Even better!


- David


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


On 2012-04-27 02:57:41, Vandana Ayyalasomayajula wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4732/
> -----------------------------------------------------------
> 
> (Updated 2012-04-27 02:57:41)
> 
> 
> 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/ivy.xml 1331188 
>   /trunk/src/java/org/apache/hcatalog/cli/HCatCli.java 1331188 
>   
> /trunk/src/java/org/apache/hcatalog/cli/SemanticAnalysis/HCatSemanticAnalyzer.java
>  1331188 
>   /trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1331188 
>   /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java 
> 1331188 
>   
> /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java
>  1331188 
>   /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java 1331188 
>   /trunk/src/java/org/apache/hcatalog/data/JsonSerDe.java 1331188 
>   
> /trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java
>  1331188 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1331188 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1331188 
>   /trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1331188 
>   /trunk/src/test/org/apache/hcatalog/cli/TestEximSemanticAnalysis.java 
> 1331188 
>   /trunk/src/test/org/apache/hcatalog/cli/TestPermsGrp.java 1331188 
>   /trunk/src/test/org/apache/hcatalog/cli/TestSemanticAnalysis.java 1331188 
>   /trunk/src/test/org/apache/hcatalog/data/HCatDataCheckUtil.java 1331188 
>   /trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1331188 
>   /trunk/src/test/org/apache/hcatalog/data/TestJsonSerDe.java 1331188 
>   /trunk/src/test/org/apache/hcatalog/data/schema/TestHCatSchemaUtils.java 
> 1331188 
>   /trunk/src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 
> 1331188 
>   
> /trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java 
> 1331188 
>   /trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 
> 1331188 
>   /trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 
> 1331188 
>   
> /trunk/src/test/org/apache/hcatalog/rcfile/TestRCFileMapReduceInputFormat.java
>  1331188 
> 
> Diff: https://reviews.apache.org/r/4732/diff
> 
> 
> Testing
> -------
> 
> Unit tests pass. Log messages verified from test logs.
> 
> 
> Thanks,
> 
> Vandana
> 
>

Reply via email to