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

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



bq.  On 2012-03-13 16:33:21, Francis Liu wrote:
bq.  > trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java, line 
120
bq.  > <https://reviews.apache.org/r/4309/diff/1/?file=91633#file91633line120>
bq.  >
bq.  >     I just noticed InputJobInfo has properties and  jobProperties. How 
are they different? If users are gonna pass implementation specific configs in 
job properties now then we should make this line a merge instead of a replace.
bq.  
bq.  Sushanth Sowmyan wrote:
bq.      I didn't remove Properties outright because I saw that the HBase 
storage driver seemed to be using it, but the intent was to replace it. In the 
latest diff, I've outright replaced it.
bq.  
bq.  Francis Liu wrote:
bq.      so this is just a rename of properties->jobProperties? we should 
probably address that in a diff patch.

No, it's not a rename, it's a addition of jobProperties/replacement of 
functionality/removal or properties. properties was a layover from 
StorageDrivers. The goal here was to streamline jobProperties from TableDesc, 
and once that's in place, there's no real further need for properties itself.

properties is also of the datatype Properties as opposed to Map<String,String>, 
which is what's needed for jobProperties. Cross-converting and merging is more 
difficult, so replacing is easier.


bq.  On 2012-03-13 16:33:21, Francis Liu wrote:
bq.  > trunk/src/java/org/apache/hcatalog/mapreduce/HCatStorageHandler.java, 
line 74
bq.  > <https://reviews.apache.org/r/4309/diff/1/?file=91632#file91632line74>
bq.  >
bq.  >     we should change output at the same time to keep things consistent.
bq.  
bq.  Sushanth Sowmyan wrote:
bq.      Oh, I thought that was the subject of another jira that you'd taken on 
at the end of the other meeting we had. Let me see if I can quickly change this 
too.
bq.  
bq.  Sushanth Sowmyan wrote:
bq.      Actually, looking at HCatUtil.configureOutputStorageHandler , other 
behaviour needs a change first - this is better handled in a separate jira, I'm 
going to create one for it.
bq.      
bq.      In that function, you seem to copy all the entries of JobContext into 
tableDesc.jobProperties. Why is that? That really shouldn't be happening as 
that bloats up jobProperties significantly, also, assuming you instantiated the 
storageHandler using ReflectionUtils, don't you already get the conf set on it? 
I guess it didn't matter when TableDesc was created and thrown away, but not if 
used for a unified jobProperties.
bq.  
bq.  Francis Liu wrote:
bq.      Yes we agreed on a separate meeting it's a bug. Jira is HCATALOG-283. 
Patch is a 3 line change. There's really no need for this to be addressed in a 
separate jira. You can include HCATALOG-283 here or just pass everything in as 
it's doing now.

Aha, so that's where  that was. :)

Ok, including, marking that as a duplicate and uploading new patch.


- Sushanth


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


On 2012-03-13 20:56:02, Sushanth Sowmyan wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4309/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-03-13 20:56:02)
bq.  
bq.  
bq.  Review request for Alan Gates and Francis Liu.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  + changing signature of configureInputJobProperties to take only 
TableDesc, and to use its jobProperties
bq.  + calling configureInputJobProperties at a table/job level, not at a 
partition level.
bq.  
bq.  
bq.  This addresses bug HCATALOG-308.
bq.      https://issues.apache.org/jira/browse/HCATALOG-308
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapred/HCatMapredInputFormat.java 
1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapred/HCatMapredOutputFormat.java 
1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapred/HiveHCatSplitWrapper.java 
1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 
1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapreduce/HCatStorageHandler.java 
1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 
1300322 
bq.    trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1300322 
bq.    
trunk/src/java/org/apache/hcatalog/storagehandler/HCatStorageHandlerImpl.java 
1300322 
bq.    trunk/src/test/org/apache/hcatalog/cli/DummyStorageHandler.java 1300322 
bq.    
trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseHCatStorageHandler.java
 1300322 
bq.    
trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestSnapshots.java
 1300322 
bq.  
bq.  Diff: https://reviews.apache.org/r/4309/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Sushanth
bq.  
bq.


                
> configureInputJobProperties(TableDesc) related changes
> ------------------------------------------------------
>
>                 Key: HCATALOG-308
>                 URL: https://issues.apache.org/jira/browse/HCATALOG-308
>             Project: HCatalog
>          Issue Type: Sub-task
>            Reporter: Sushanth Sowmyan
>            Assignee: Sushanth Sowmyan
>             Fix For: 0.4
>
>         Attachments: HCATALOG-308.2.patch, HCATALOG-308.patch
>
>


--
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