----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34925/#review88171 -----------------------------------------------------------
Mostly look fine. A couple of questions - - Does the sentry config file need to be in classpath ? can it be specified via another argument ? - I guess it will need some work for secure connection. The DB client needs the actions to be wrapped in security context. In case of Hive or other service, they already have secure UGI object created. In case of the shell we'll need to handle it. - We should definately log a followup ticket for generic privileges. Few minor comments/suggestions below bin/sentryShell (line 1) <https://reviews.apache.org/r/34925/#comment140578> It would be good to have an option to invoke this from sentry script itself. Perhaps you can log a followup ticket to address that in future. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java (line 238) <https://reviews.apache.org/r/34925/#comment140579> Isn't this already verified on the server side. In that case, we don't have duplicate the validation logic in shell again. - Prasad Mujumdar On June 2, 2015, 5:09 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34925/ > ----------------------------------------------------------- > > (Updated June 2, 2015, 5:09 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/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/34925/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
