[ 
https://issues.apache.org/jira/browse/HCATALOG-373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13276194#comment-13276194
 ] 

[email protected] commented on HCATALOG-373:
--------------------------------------------------------



bq.  On 2012-05-11 23:42:33, Francis Liu wrote:
bq.  > 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.
bq.  
bq.  Travis Crawford wrote:
bq.      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?
bq.  
bq.  Francis Liu wrote:
bq.      Cool. I don't mind either way.
bq.  
bq.  Travis Crawford wrote:
bq.      Hey Francis,
bq.      
bq.      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.
bq.  
bq.  Francis Liu wrote:
bq.      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.
bq.  
bq.  Travis Crawford wrote:
bq.      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.
bq.      
bq.      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.

I see, I'm no fan either and since you're addressing it in a separate jira. I'd 
rather we make the changes there rather than making it point to potentially 
another external directory. I'll check this in without this section. Let me 
know if you're ok with that.


- Francis


-----------------------------------------------------------
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:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4971/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-05-05 01:23:50)
bq.  
bq.  
bq.  Review request for hcatalog and Francis Liu.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  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.
bq.  
bq.  
bq.  This addresses bug HCATALOG-373.
bq.      https://issues.apache.org/jira/browse/HCATALOG-373
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 268167e 
bq.    src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 65f96f4 
bq.    src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1837081 
bq.    src/java/org/apache/hcatalog/mapreduce/ProgressReporter.java fb379cd 
bq.    src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java f3d07a0 
bq.  
bq.  Diff: https://reviews.apache.org/r/4971/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  "ant clean test" passes
bq.  
bq.  I can run pig+hcatalog queries using Elephant-Bird deprecated API 
wrappers, which is why this issue originally came up.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Travis
bq.  
bq.


                
> ProgressReporter should work with both old and new MR API
> ---------------------------------------------------------
>
>                 Key: HCATALOG-373
>                 URL: https://issues.apache.org/jira/browse/HCATALOG-373
>             Project: HCatalog
>          Issue Type: Bug
>            Reporter: Travis Crawford
>            Assignee: Travis Crawford
>         Attachments: HCATALOG-373_progress_reporter.diff, 
> HCATALOG-373_progress_reporter_2.diff, HCATALOG-373_progress_reporter_3.diff, 
> HCATALOG-373_progress_reporter_4.diff
>
>
> {{org.apache.hcatalog.mapreduce.ProgressReporter}} currently implements 
> {{org.apache.hadoop.mapred.Reporter}}. It should also extend 
> {{org.apache.hadoop.mapreduce.StatusReporter}} so it works with code 
> expecting either an old or new API reporter.
> The use case is using a wrapper so a serde works with a new-API input format.
> https://github.com/kevinweil/elephant-bird/blob/master/src/java/com/twitter/elephantbird/mapred/input/DeprecatedInputFormatWrapper.java#L163

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to