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

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



bq.  On 2012-05-08 16:41:27, David Capwell wrote:
bq.  > 
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerConfiguration.java,
 line 64
bq.  > <https://reviews.apache.org/r/5062/diff/1/?file=107664#file107664line64>
bq.  >
bq.  >     Why use merge when you can just use the Configuration constructor?  
The constructor should also be faster.
bq.  
bq.  Francis Liu wrote:
bq.      It's for cases where I'd like to modify an instance as opposed to 
creating a new one.
bq.  
bq.  David Capwell wrote:
bq.      You are not doing that though, so why not just use the Constructor and 
if the need comes up then implement it.
bq.  
bq.  Francis Liu wrote:
bq.      Actually I am doing that :-). See the create(Configuration) method.

You are doing this in create(Config)

 Configuration conf = create(); // creates a new instance
 merge(conf, that);

Merge adds everything from the right side into the left.  So in this case the 
newly created configuration object.  Since this is the only time merge is used 
why not just use the constructor?  

Configuration conf = new Configuration(that); 
addResources(conf); 
return conf;  

Doing the above is faster than enumerating over the list + method calls and is 
more clear what you are trying to do.


- David


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


On 2012-05-08 05:45:38, Francis Liu wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/5062/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-05-08 05:45:38)
bq.  
bq.  
bq.  Review request for hcatalog, Vandana Ayyalasomayajula and Thomas.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  revision manager currently expects some configs to be included in 
hbase-site.xml, which complicates deployment as well as adding unrelated 
properties which are not native hbase.
bq.  
bq.  I've added a new class and revision-*.xml artifacts.
bq.  
bq.  
bq.  This addresses bug HCATALOG-404.
bq.      https://issues.apache.org/jira/browse/HCATALOG-404
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    storage-handlers/hbase/build.xml 6653a66 
bq.    storage-handlers/hbase/conf/revision-manager-site.xml PRE-CREATION 
bq.    
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseHCatStorageHandler.java
 d3faa04 
bq.    
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerConfiguration.java
 PRE-CREATION 
bq.    
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java
 55c97e0 
bq.    
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java
 3e03909 
bq.    
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java
 cffcf5e 
bq.    storage-handlers/hbase/src/resources/revision-manager-default.xml 
PRE-CREATION 
bq.    storage-handlers/hbase/src/test/log4j.xml 8fd4fc9 
bq.    
storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerConfiguration.java
 PRE-CREATION 
bq.    
storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java
 1e94fd2 
bq.  
bq.  Diff: https://reviews.apache.org/r/5062/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Francis
bq.  
bq.


                
> add separate configuration for revision manager
> -----------------------------------------------
>
>                 Key: HCATALOG-404
>                 URL: https://issues.apache.org/jira/browse/HCATALOG-404
>             Project: HCatalog
>          Issue Type: Improvement
>    Affects Versions: 0.4.1
>            Reporter: Francis Liu
>            Assignee: Francis Liu
>         Attachments: HCATALOG-404.patch
>
>
> revision manager currently expects some configs to be included in 
> hbase-site.xml, which is complicates deployment and adding unrelated 
> properties which are not native hbase.

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