Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Hive, mail # dev - Review Request 17441: Adds concept of root role in Hive.


Copy link to this message
-
Re: Review Request 17441: Adds concept of root role in Hive.
Thejas Nair 2014-01-28, 03:07

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

common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/17441/#comment61984>

    we should add this to conf/hive-default.xml.template and document it there.
    Can you also put the same into the release notes section of jira ?
    

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestEmbeddedHiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61986>

    can you also test if the users from config are getting added to the role ?
    

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61985>

    use ALL CAPS for final string variable name ?
    Should role name also be all CAPS ? The PUBLIC role is usually mentioned in CAPS, since this is a similar special role, using cap letters might make sense.
    
    

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61978>

    you can make this final as well, as the object reference should not change

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61976>

    Can you fix the indentation ?

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61979>

    how about logging this in info or warn ? As this borders on an error condition.
    

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61980>

    LOG.warn would be better

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61981>

    it would be useful to also print the value that this code saw (userStr), it might be useful while debugging.
    Also, LOG.warn or error might be more appropriate, as the user has given an invalid value.

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61982>

    LOG.error here?
    

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/17441/#comment61983>

    LOG.error
- Thejas Nair
On Jan. 28, 2014, 2:03 a.m., Ashutosh Chauhan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17441/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2014, 2:03 a.m.)
>
>
> Review request for hive and Thejas Nair.
>
>
> Bugs: HIVE-5959
>     https://issues.apache.org/jira/browse/HIVE-5959
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Adds concept of root role in Hive.
>
>
> Diffs
> -----
>
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 371cb0f
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestEmbeddedHiveMetaStore.java b6b0e6e
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 58f9957
>
> Diff: https://reviews.apache.org/r/17441/diff/
>
>
> Testing
> -------
>
> New junit test added.
>
>
> Thanks,
>
> Ashutosh Chauhan
>
>