----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26011/#review54493 -----------------------------------------------------------
Ship it! sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java <https://reviews.apache.org/r/26011/#comment94677> since the roles are case-sensitive, do you need to call .toLower() on "role"? Might be good to add a test case for this. sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift <https://reviews.apache.org/r/26011/#comment94678> comment on when this wouldn't be set... Should we really change it to optional or should it be set to an empty map if nothing matches? sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java <https://reviews.apache.org/r/26011/#comment94679> Fix spelling of Authrizable - Lenni Kuff On Sept. 25, 2014, 1:36 a.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26011/ > ----------------------------------------------------------- > > (Updated Sept. 25, 2014, 1:36 a.m.) > > > Review request for sentry, Lenni Kuff and Sravya Tirukkovalur. > > > Bugs: SENTRY-469 > https://issues.apache.org/jira/browse/SENTRY-469 > > > Repository: sentry > > > Description > ------- > > list_sentry_privileges_by_authorizable API should support impersonation > similar to other Sentry service RPCs. > - Add requester username as required argument for the API > - Verify the admin status of the requesting user > - Validate that the requesting user is part of groups and the roles it's > trying to access > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > 0668912 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > e3cdfc2 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > d8357aa > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 38cb39b > > Diff: https://reviews.apache.org/r/26011/diff/ > > > Testing > ------- > > Added test case to validated various options of the > list_sentry_privileges_by_authorizable() for non-admin user. > > > Thanks, > > Prasad Mujumdar > >
