----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34086/#review127555 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java (line 107) <https://reviews.apache.org/r/34086/#comment190918> Use null instead of new HashSet<String>(). The same for the following instances. One question to consider: Is the original version of listPrivilegesForProvider without users parameters still be meaningful? If the answer is YES, then we can add an overload version with users instead of directly changing the original one with users parameter. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java (line 134) <https://reviews.apache.org/r/34086/#comment190920> Can we also use ADMIN_USER to construct the userNames. requestorUserNames = Sets.newHashSet(ADMIN_USER); And perform grantRoleToUser similar to group to test both groups and users in this case instead of passing the empty user list. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java (line 140) <https://reviews.apache.org/r/34086/#comment190921> Can we also use ADMIN_USER to construct the userNames. requestorUserNames = Sets.newHashSet(ADMIN_USER); And perform grantRoleToUser similar to group to test both groups and users in this case instead of passing the empty user list. - Jerry Chen On April 6, 2016, 2:56 p.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34086/ > ----------------------------------------------------------- > > (Updated April 6, 2016, 2:56 p.m.) > > > Review request for sentry, Dapeng Sun and Jerry Chen. > > > Repository: sentry > > > Description > ------- > > Update SentryPolicyStoreProcessor for grant user to role > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > de50adb > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java > edc5661 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 8881d82 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java > ac4df77 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java > 0792eb6 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 07c7f7a > > Diff: https://reviews.apache.org/r/34086/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
