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