----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42682/#review116263 -----------------------------------------------------------
Thanks for the patch, it looks good to me overall with some minor comments. I'm wonder if no update for sentryShell, how this feature works for Solr with command line? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java (line 68) <https://reviews.apache.org/r/42682/#comment177279> The comments for code can be removed. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java (line 114) <https://reviews.apache.org/r/42682/#comment177280> Currently, only collection for Solr, and validate this already in the previous code: if (authz instanceof Collection) { .... } else { throw new IllegalArgumentException } This method is not necessary for current model. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java (line 267) <https://reviews.apache.org/r/42682/#comment177281> white spaces sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java (line 285) <https://reviews.apache.org/r/42682/#comment177282> white spaces. - Colin Ma On Jan. 26, 2016, 12:10 a.m., Gregory Chanan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42682/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2016, 12:10 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Here is a solr shell, based on the existing hive shell. Actually most of the > work is specific to the generic model, there is only a small amount of > solr-specific code. So it should be easily extensible for other clients. > > Some limitations: > > It is not integrated with bin/sentryShell yet > It does not support any grant options > It is pretty restrictive in what it supports, i.e. it checks for know > authorizables and throws an exception if it doesn't know about any. So it > can't be used for anything outside of collection permissions. > > > Diffs > ----- > > sentry-provider/sentry-provider-db/pom.xml > 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/Command.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/CreateRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/DropRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GrantPrivilegeToRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/ListPrivilegesByRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/ListRolesCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/RevokePrivilegeFromRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConvertor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java > b1353c5312e9d53b6dd184edd1fa8c11a92b4374 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceIntegrationBase.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > 4732ea2fcac1f9cb6a885592378f7916edbde3a1 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/42682/diff/ > > > Testing > ------- > > Ran the unit tests. > > > Thanks, > > Gregory Chanan > >
