----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67293/#review203889 -----------------------------------------------------------
sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 338 (patched) <https://reviews.apache.org/r/67293/#comment286276> The name should be more generic. In the future, it can be used for other purpose, not just for owner privilege creation or transfer sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 339 (patched) <https://reviews.apache.org/r/67293/#comment286247> do we need to increase the service version number? The API has changed with new request sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 341 (patched) <https://reviews.apache.org/r/67293/#comment286277> how are you going to make sure the client and server use the same string for a given event type? Should it be a enum type instead of string? sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 343 (patched) <https://reviews.apache.org/r/67293/#comment286278> Can you change the field name from "owner" to "ownerName"? then you don't need comment sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 344 (patched) <https://reviews.apache.org/r/67293/#comment286279> how about this one from "authorizable" to "newAuthorizable"? sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 349 (patched) <https://reviews.apache.org/r/67293/#comment286280> if sync notification ID succeeds and create owner privilege fail. Will the status be success or failure? sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift Lines 393 (patched) <https://reviews.apache.org/r/67293/#comment286281> Can you change the wording to "Synchronize between HMS notifications and Sentry and update owner info and also" to reflect the order of the execution - Na Li 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 > >
