----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4971/#review7585 -----------------------------------------------------------
Looks good. Some small nitpicks. Would it be possible to add or augment an existing junit test to verify that the counters are updated correctly? src/java/org/apache/hcatalog/common/HCatUtil.java <https://reviews.apache.org/r/4971/#comment16770> can we move this to internal util? src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java <https://reviews.apache.org/r/4971/#comment16768> why not use hcatSplit here? src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java <https://reviews.apache.org/r/4971/#comment16769> and here? - Francis On 2012-05-02 17:55:29, Travis Crawford wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4971/ > ----------------------------------------------------------- > > (Updated 2012-05-02 17:55:29) > > > Review request for hcatalog and Francis Liu. > > > Summary > ------- > > Update ProgressReporter to work with both old and new mapreduce API. Delay > creating the base record reader so we have a StatusReporter and can use > counters. > > > This addresses bug HCATALOG-373. > https://issues.apache.org/jira/browse/HCATALOG-373 > > > Diffs > ----- > > src/java/org/apache/hcatalog/common/HCatUtil.java cb6404a > src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 268167e > src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 65f96f4 > src/java/org/apache/hcatalog/mapreduce/ProgressReporter.java fb379cd > > Diff: https://reviews.apache.org/r/4971/diff > > > Testing > ------- > > "ant clean test" passes > > I can run pig+hcatalog queries using Elephant-Bird deprecated API wrappers, > which is why this issue originally came up. > > > Thanks, > > Travis > >
