----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67402/#review204151 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 793 (patched) <https://reviews.apache.org/r/67402/#comment286539> typo: Privielge Also, add more information about why this is done this way instead of using the contains(). You can mention that the inputPrivilege contains all fields except the roles and users information and we are finding the privilege with all users and roles that matches the inputPrivilege. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 792 (original), 813 (patched) <https://reviews.apache.org/r/67402/#comment286540> Is there some tests that verify this method or the parent public method? We need to make sure this new logic works as expected. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 814 (original), 835-836 (patched) <https://reviews.apache.org/r/67402/#comment286538> Shoult it be better to move this method to the MSentryRole class instead? This would look better: MSentryPrivilege mSelect = mRole.findPrivilege(convertToMSentryPrivilege(tNotAll)); - Sergio Pena On May 31, 2018, 3:49 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67402/ > ----------------------------------------------------------- > > (Updated May 31, 2018, 3:49 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda and Sergio Pena. > > > Bugs: sentry-2244 > https://issues.apache.org/jira/browse/sentry-2244 > > > Repository: sentry > > > Description > ------- > > When granting privilege to a user or role, sentry does not need to get > individual privilege with specific action from DB again after all its > associated privileges are fetched from DB. We just just find the > corresponding privilege from list to avoid extra query to DB > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 5932335 > > > Diff: https://reviews.apache.org/r/67402/diff/1/ > > > Testing > ------- > > test cases in TestSentryStore succeeded > > > Thanks, > > Na Li > >
