----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26984/#review57677 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/26984/#comment98492> why is this better? seems you can do everything in the first list. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/26984/#comment98494> is there a config var for the blacklist too? Do you have to add the whitelist param to the blacklist by default? common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/26984/#comment98495> do we document that the blacklist overwrites the whitelist? common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/26984/#comment98496> given that the pattern can be null, why do you need an extra boolean? even simpler: set pattern to ".*" by default and don't allow it to be null. (if you want to enable specific checking, write your own regex). ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/SettableConfigUpdater.java <https://reviews.apache.org/r/26984/#comment98510> I'm missing something here. This seems overly complicated. You have two copies of the same list of parameters and the actual set method does a bunch of conversion (. -> \. etc). Why not just add these as the default to the HiveConf class? ql/src/test/queries/clientnegative/authorization_disallow_transform.q <https://reviews.apache.org/r/26984/#comment98511> do you want to add a .q file test that checks setting of variables too? - Gunther Hagleitner On Oct. 21, 2014, 6:31 p.m., Thejas Nair wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26984/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2014, 6:31 p.m.) > > > Review request for hive, Gunther Hagleitner and Jason Dere. > > > Bugs: HIVE-8534 > https://issues.apache.org/jira/browse/HIVE-8534 > > > Repository: hive-git > > > Description > ------- > > https://issues.apache.org/jira/browse/HIVE-8534 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a9da7d > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/SettableConfigUpdater.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java > 658ff76 > > ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/TestSQLStdHiveAccessControllerHS2.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/TestSQLStdHiveAccessControllerHS2.java > f13cf7e > ql/src/test/queries/clientnegative/authorization_disallow_transform.q > 342c29a > ql/src/test/results/clientnegative/authorization_disallow_transform.q.out > 39819b6 > > Diff: https://reviews.apache.org/r/26984/diff/ > > > Testing > ------- > > Tests updated > > > Thanks, > > Thejas Nair > >
