----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67134/#review204851 -----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java Lines 288 (patched) <https://reviews.apache.org/r/67134/#comment287596> Space between >users sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java Lines 492 (patched) <https://reviews.apache.org/r/67134/#comment287597> Do we need a similar test for SHOW GRANT ... ON DATABASE ... ? I don't see that test exists. sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java Line 241 (original), 259 (patched) <https://reviews.apache.org/r/67134/#comment287600> Is there a way to have the same method listPrivilegesbyAuthorizable() but that returns privileges for any entity? This to avoid making several calls in the future for privs for roles, privs for user, privs for groups? Having a class that wraps different maps per entity would be useful. sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java Lines 263 (patched) <https://reviews.apache.org/r/67134/#comment287599> What about a javadoc comment in this method here while you're modifying them? sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java Lines 941-942 (original), 960-961 (patched) <https://reviews.apache.org/r/67134/#comment287603> This line does not have any change except a space or tab. We should not include this change on the commit so we keep the history of who made this line change. sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java Line 955 (original), 974 (patched) <https://reviews.apache.org/r/67134/#comment287604> Same tab space here. We should remove the tab change. - Sergio Pena On June 13, 2018, 10:10 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67134/ > ----------------------------------------------------------- > > (Updated June 13, 2018, 10:10 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and > Sergio Pena. > > > Repository: sentry > > > Description > ------- > > Currently Sentry doesn't support Hive command to show privileges on > authorizables without mentioning any role or user name > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java > 23246c903 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java > f0b4b4466 > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java > 2e3fd7f36 > > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java > 6f38ed20d > > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java > 4e605ae78 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestShowGrants.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/67134/diff/4/ > > > Testing > ------- > > $ mvn -f sentry-binding/pom.xml test > $ mvn -f sentry-provider/pom.xml test > > > Thanks, > > Arjun Mishra > >
