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



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreCommand.java
<https://reviews.apache.org/r/669/#comment1265>

    The example in this Javadoc needs to be updated to match the new invocation 
pattern.  (Also, the original Javadoc was missing <pre> and <code> tags; can 
you add those in here so that the HTML will get generated correctly?)
    
    But first...is the new invocation pattern actually better?  It seems like 
the invocation has gotten a little more verbose since each call to 
executeWithRetry now has to pass in two arguments.  Wouldn't it be better to 
leave the executeWithRetry method in the HiveMetaStore class so that it can do 
any common stuff like setting these as attributes on the command object?
    
    Likewise, I'm not clear on how moving the thread-local access to the 
command object is an improvement.  Seems to me it should stay in HiveMetaStore; 
again, you can pass the right object in from executeWithRetry.
    


- John


On 2011-04-27 18:24:42, Ashutosh Chauhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/669/
> -----------------------------------------------------------
> 
> (Updated 2011-04-27 18:24:42)
> 
> 
> Review request for hive, Carl Steinbach, John Sichi, and Paul Yang.
> 
> 
> Summary
> -------
> 
> See HIVE-2135
> 
> 
> This addresses bug HIVE-2135.
>     https://issues.apache.org/jira/browse/HIVE-2135
> 
> 
> Diffs
> -----
> 
>   
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 1096976 
>   
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreCommand.java
>  PRE-CREATION 
>   
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 1096976 
>   
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/URLConnectionUpdater.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/669/diff
> 
> 
> Testing
> -------
> 
> Since this is a refactoring patch, no new tests are required. Ran all the 
> tests in metastore. All of them passed.
> 
> 
> Thanks,
> 
> Ashutosh
> 
>

Reply via email to