----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14447/#review26631 -----------------------------------------------------------
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/14447/#comment51874> Can you static import this instead of using the package name? There are a couple other occurrences of this in the same file that would be nice to clean up. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/14447/#comment51873> I think we should try to make security-related configuration properties easier to find by using an appropriate prefix, e.g. hive.security.* Also, "whitelist" and "blacklist" are the standard terms used by security engineers, so I think this property should be named hive.security.command.whitelist. It seems like this property should be a default member of hive.conf.restricted.list. conf/hive-default.xml.template needs to be update. ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java <https://reviews.apache.org/r/14447/#comment51875> There should be a space between 'if' and '('. The same rule also applies to for statements. This same formatting problem is also found in the other files included in this patch. Refs: * https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConvention * http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#430 - Carl Steinbach On Oct. 2, 2013, 6:04 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14447/ > ----------------------------------------------------------- > > (Updated Oct. 2, 2013, 6:04 p.m.) > > > Review request for hive and Edward Capriolo. > > > Bugs: HIVE-5400 > https://issues.apache.org/jira/browse/HIVE-5400 > > > Repository: hive-git > > > Description > ------- > > Allows admins to restrict the commands, non-sql, available to users. > > > Diffs > ----- > > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java ed71196 > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5dfcff4 > hwi/src/java/org/apache/hadoop/hive/hwi/HWISessionItem.java a10c3a8 > > ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java > 0d0bf47 > ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java > PRE-CREATION > > service/src/java/org/apache/hive/service/cli/operation/AddResourceOperation.java > fe0c6db > > service/src/java/org/apache/hive/service/cli/operation/DeleteResourceOperation.java > 496bba9 > service/src/java/org/apache/hive/service/cli/operation/DfsOperation.java > a8b8ed4 > > service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java > 6b5a5c3 > > service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java > 0a8825e > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > 1f78a1d > service/src/java/org/apache/hive/service/cli/operation/SetOperation.java > bf6969a > > Diff: https://reviews.apache.org/r/14447/diff/ > > > Testing > ------- > > New unit tests are added and pass. > > > Thanks, > > Brock Noland > >