----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21784/#review43648 -----------------------------------------------------------
Ship it! Looks fine to me. I think the group to role is handled correctly. This issue is only for role to privileges. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java <https://reviews.apache.org/r/21784/#comment77930> so I guess the schema changes done in SENTRY-209 does take care of m to n relationship for roles and privileges automatically. We can remove this TODO. - Prasad Mujumdar On May 21, 2014, 6:58 p.m., Arun Suresh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21784/ > ----------------------------------------------------------- > > (Updated May 21, 2014, 6:58 p.m.) > > > Review request for sentry, Prasad Mujumdar and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > Looks like we were doing an extra privilege append on the role inside the > SentryStore. The 'privilege.appendRole(role)' already calls > 'role.appendPrivilege(privilege)'. > It Looks like DataNucleus does some byte-code weaving to intercept the append > call and tries to do some other magic.. which is why I guess it fires extra > select queries > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > 4030205 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java > e375a4c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > cd71a58 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > a2e877a > > Diff: https://reviews.apache.org/r/21784/diff/ > > > Testing > ------- > > > Thanks, > > Arun Suresh > >
