----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67293/#review203899 -----------------------------------------------------------
sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 41 (patched) <https://reviews.apache.org/r/67293/#comment286271> Should we rename this TSentryOwnerType? What if we decide to add another type of ownership in a different model, like generic? ROLE and USER is generic for all models. sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 338 (patched) <https://reviews.apache.org/r/67293/#comment286266> Same comment as the method name. We should have a different name in case we want to extend the implementation of the method in the future. Let's find something more generic. sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 340 (patched) <https://reviews.apache.org/r/67293/#comment286267> Don't we need commas in this line and the rest of the fields? sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 344-345 (patched) <https://reviews.apache.org/r/67293/#comment286270> Isn't it better to make the authorizable in the sentry side instead? For instance, how does the client know about the oldAuthorizable? What if the client adds a different user or role in the oldAuthorizable? That will keep the old owner authorized in the sentry server. sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 394 (patched) <https://reviews.apache.org/r/67293/#comment286265> We should come up with a different name for this method in case we want to extend it in the future. - Sergio Pena On May 24, 2018, 11:18 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67293/ > ----------------------------------------------------------- > > (Updated May 24, 2018, 11:18 p.m.) > > > Review request for sentry, Na Li and Sergio Pena. > > > Bugs: SENTRY--2243 > https://issues.apache.org/jira/browse/SENTRY--2243 > > > Repository: sentry > > > Description > ------- > > When ever below events happen sentry should be updated with owner information > so that owner privileges can be updated accordingly. > > Create/drop database > Create/drop table > Alter database/table > This needs new messages types, requests and responses to be added. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > 816cfe182a9d42ba9b553ab10a149171b28d3727 > > sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java > fbbd725d31bccba516c266175bc8f993f4f3287f > > sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryHmsObjectOwnerType.java > PRE-CREATION > > sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryOwnerUpdateAndSyncIDRequest.java > PRE-CREATION > > sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryOwnerUpdateAndSyncIDResponse.java > PRE-CREATION > > sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift > dcbd7e488bab46cce1b1369bb92c4549384ef38a > > > Diff: https://reviews.apache.org/r/67293/diff/2/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
