> On 2012-05-11 23:42:33, Francis Liu wrote: > > Looks good to me. Just a concern with the change in the test output > > directory. If you agree with any of my proposals, I can change that on > > commit. Will run e2e tests. > > Travis Crawford wrote: > Sure, either approach sounds fine. The main thing is being able to run in > the IDE. Do you want to make the change at commit time or for me to make it? > > Francis Liu wrote: > Cool. I don't mind either way. > > Travis Crawford wrote: > Hey Francis, > > What do you think about checking this in as-is, and we can address the > logs directory in https://issues.apache.org/jira/browse/HCATALOG-374 ? > There's a lot of inconsistent setting up of a test metastore and I'd like to > consolidate that setup code in a base test class so updates like these are > available to all tests. I posted a patch in that issue you can look at as an > example of what I'm proposing. > > Francis Liu wrote: > Hi Travis, if you're addressing this issue in a separate Jira. I'd prefer > not checking in this portion of the patch as it might affect the automated > builds.
The tests pass locally so CI should not be affected. In general I'm not a fan of builds producing things outside the build directory, because it makes locating test data and logs more difficult, and the clean target needs to do more than simply deleting the build directory, so there are opportunities for clean to miss things. Putting files in directories with the test class name ensures state does not get shared between tests. Please let me know what you'd like me to do here so we can wrap this up. This patch has been going for over a month now and I'd really like to reach closure and move on to new issues. - Travis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4971/#review7816 ----------------------------------------------------------- On 2012-05-05 01:23:50, Travis Crawford wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4971/ > ----------------------------------------------------------- > > (Updated 2012-05-05 01:23:50) > > > 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/mapreduce/HCatBaseInputFormat.java 268167e > src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 65f96f4 > src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1837081 > src/java/org/apache/hcatalog/mapreduce/ProgressReporter.java fb379cd > src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java f3d07a0 > > 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 > >
