----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18938/#review36601 -----------------------------------------------------------
Thanks for the review. Please see my response below and let me know what you think. I'll post a new patch with the review feedback. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java <https://reviews.apache.org/r/18938/#comment67637> Please see the comment below. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java <https://reviews.apache.org/r/18938/#comment67636> Yup. How about we use an iterator[1] instead to remove the item or use Apache Commons filter? [1] - http://docs.oracle.com/javase/tutorial/collections/interfaces/collection.html sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java <https://reviews.apache.org/r/18938/#comment67638> I had this before I started to validate the privilege in the caller. I think we should just remove this. - Shreepadma Venugopalan On March 8, 2014, 2:12 a.m., Shreepadma Venugopalan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18938/ > ----------------------------------------------------------- > > (Updated March 8, 2014, 2:12 a.m.) > > > Review request for sentry, Brock Noland and Prasad Mujumdar. > > > Bugs: SENTRY-126 > https://issues.apache.org/jira/browse/SENTRY-126 > > > Repository: sentry > > > Description > ------- > > This patch, > > a) implements apis needed to grant and revoke privileges to roles > b) adds a new test case that passes > c) cleans up the thrift interface > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/SentryPolicyService.java > 04e6655 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleAddGroupsRequest.java > 034bfef > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeResponse.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TCreateSentryPrivilegeRequest.java > 34689fc > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TCreateSentryPrivilegeResponse.java > a92698e > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java > 32370ba > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java > 37f9fb7 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/service/thrift/sentry_common_serviceConstants.java > 8174fe2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > 549a9db > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java > 28416e0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryInvalidInputException.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 7419a0d > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java > dbd8dae > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java > 2425ac3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > aa83ffd > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 55dbcdd > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java > 0b2daf3 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_common_service.thrift > ed0ebc5 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > 40f8a5f > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java > dab26e1 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 692dbfa > > Diff: https://reviews.apache.org/r/18938/diff/ > > > Testing > ------- > > New test case passes. Existing tests pass. > > > Thanks, > > Shreepadma Venugopalan > >
