----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67452/#review204552 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 796 (original), 799 (patched) <https://reviews.apache.org/r/67452/#comment287114> You mean 'User does not exist', right? Might sounds like you haven't confirmed it does not exist, but you already did. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 809-815 (original), 814-820 (patched) <https://reviews.apache.org/r/67452/#comment287116> Are we going to allow that a role gets the all privilege granted if the owner privilege is already set? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 827-835 (original), 832-840 (patched) <https://reviews.apache.org/r/67452/#comment287115> Should we do the same thing for OWNER? Are we going to allow a role is granted insert/select while having the owner privilege? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 1942 (patched) <https://reviews.apache.org/r/67452/#comment287117> New line here sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2048-2051 (patched) <https://reviews.apache.org/r/67452/#comment287120> It needs better description of the method, parameters and throw exceptions in this comment. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2053 (patched) <https://reviews.apache.org/r/67452/#comment287119> How is this class going to be used? listSentryPrivilegesByAuthorizable() is used to return all privileges from an authorizable. This may be used by the SHOW GRANT ON <OBJECT> command, right? What about this method? - Sergio Pena On June 11, 2018, 3:41 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67452/ > ----------------------------------------------------------- > > (Updated June 11, 2018, 3:41 p.m.) > > > Review request for sentry, Na Li and Sergio Pena. > > > Bugs: SENTRY-2257 > https://issues.apache.org/jira/browse/SENTRY-2257 > > > Repository: sentry > > > Description > ------- > > Implement functionality in sentry store to update owner privilege on an > authorizable. > > Here is the approach. > > There are two new API's that are exposed. > > To list the owner privileges granted to an authorizable > 1. update the owner privilege to new owner > > Here is the Flow. > 1. SentryPolicyStoreProcessor would first get the list of privileges that are > to be revoked. > 2. Using the list of privileges that are to be revoked, list of > PermissionsUpdate is generated using SentryPlug-in > 3. SentryPolicyStoreProcessor would then use the new API to update the owner > privileges. > > This way all the updated listed below happen in the same transaction > 1. Revoking the exixting owner privilage for authorizable > 2. Granting new owner privilege fot authorizable. > 3. Adding delta update for owner privilege revoked > 4. Adding delta update for owner privilege granted. > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f0e373a9aa5342d1b507e8b192cfdbc444242227 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > c056446262ddcf61db307eafbb785eac42973c80 > > > Diff: https://reviews.apache.org/r/67452/diff/7/ > > > Testing > ------- > > Added new tests to verify new functionality added. > > > Thanks, > > kalyan kumar kalvagadda > >
