> On Dec. 22, 2015, 2:26 a.m., Gregory Chanan wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java, > > line 19 > > <https://reviews.apache.org/r/34925/diff/2/?file=1165630#file1165630line19> > > > > Everything below here looks hive specific. Put in > > sentry.provider.db.tools.hive package? > > Colin Ma wrote: > Yes, it's for hive only and the class name is suffix with Hive. But for > the package org.apache.sentry.provider.db.tools.command, I move it to > org.apache.sentry.provider.db.tools.command.**hive** as your suggestion, > thanks.
looks good to me, thanks. > On Dec. 22, 2015, 2:26 a.m., Gregory Chanan wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java, > > line 196 > > <https://reviews.apache.org/r/34925/diff/2/?file=1165629#file1165629line196> > > > > Missing parameter: + paramName > > Colin Ma wrote: > Thanks, this is updated as Missing required option: as the same as the > error message from GnuParser.parse. looks good! > On Dec. 22, 2015, 2:26 a.m., Gregory Chanan wrote: > > bin/sentryShell, line 53 > > <https://reviews.apache.org/r/34925/diff/2/?file=1165628#file1165628line53> > > > > why are you using HADOOP_CLASSPATH? Where is that used? > > Colin Ma wrote: > The shell is started by the command like: > exec $HADOOP jar ${SENTRY_HOME}/lib/${_CMD_JAR} > org.apache.sentry.SentryShellHive ${args[@]} > HADOOP_CLASSPATH is for this command. Why execute it through the hadoop command rather than having it be a stand alone shell? > On Dec. 22, 2015, 2:26 a.m., Gregory Chanan wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java, > > line 67 > > <https://reviews.apache.org/r/34925/diff/2/?file=1165629#file1165629line67> > > > > At least with the file-based backend, a role was made up of groups. So > > adding a role to group is backwards. Am I missing something? > > Colin Ma wrote: > The target for this tools is only for database-based backend. For the > file-based, I think modify the configuration file is more convenient and it > isn't supported by this tool. I think you misunderstand. I'm not saying this needs to work for the file-based backend. I'm saying that it should be "add group to role" not "add role to group". Put another way: A group is a set of users. A role is a set of groups. You don't say "add group to user" you say "add user to group" So by analogy, you don't say "add role to group" you say "add group to role" - Gregory ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34925/#review111569 ----------------------------------------------------------- On Dec. 23, 2015, 8:42 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34925/ > ----------------------------------------------------------- > > (Updated Dec. 23, 2015, 8:42 a.m.) > > > Review request for sentry, Dapeng Sun, shen guoquan, and Prasad Mujumdar. > > > Repository: sentry > > > Description > ------- > > Create a simpler shell for sentry that handles command line arguments, eg > sentryShell --grant_role_privilege --role analyst --privilege > server=server1->database=db2->table=tab1->action=select > > > Diffs > ----- > > bin/sentryShell PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/Command.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CreateRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/DropRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/GrantPrivilegeToRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/GrantRoleToGroupsCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/ListPrivilegesCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/ListRolesCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/RevokePrivilegeFromRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/RevokeRoleFromGroupsCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java > 6bc9f75 > > Diff: https://reviews.apache.org/r/34925/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
