> On Dec. 8, 2015, 8:41 a.m., Hao Hao wrote: > > bin/sentryShell, line 1 > > <https://reviews.apache.org/r/34925/diff/1/?file=976349#file976349line1> > > > > Could you please also fix the unittest?
The test failed is not caused by this patch, I created SENTRY-981 for the test failed. > On Dec. 8, 2015, 8:41 a.m., Hao Hao wrote: > > bin/sentryShell, line 43 > > <https://reviews.apache.org/r/34925/diff/1/?file=976349#file976349line43> > > > > Should we also make _CMD_JAR user definable? Good point, update the patch and SENTRY_SHELL_JAR can be used as user definable. > On Dec. 8, 2015, 8:41 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java, > > line 62 > > <https://reviews.apache.org/r/34925/diff/1/?file=976350#file976350line62> > > > > change the "create role" comment to "sentry config file path". thanks for catch it. > On Dec. 8, 2015, 8:41 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java, > > line 420 > > <https://reviews.apache.org/r/34925/diff/1/?file=976352#file976352line420> > > > > Should we assert the exception type? Good point, I'll use the specific exception(eg, SentryUserException) instead of Exception. > On Dec. 8, 2015, 8:41 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java, > > line 199 > > <https://reviews.apache.org/r/34925/diff/1/?file=976350#file976350line199> > > > > Will the required option be speficed as required in the printed usage? If required option is missed, there will be a ParseException thrown to warn the user. > On Dec. 8, 2015, 8:41 a.m., Hao Hao wrote: > > bin/sentryShell, line 58 > > <https://reviews.apache.org/r/34925/diff/1/?file=976349#file976349line58> > > > > Remove all the extra spaces. done - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34925/#review109296 ----------------------------------------------------------- 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 > >
