> On 十月 5, 2014, 3:01 p.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift, > > line 109 > > <https://reviews.apache.org/r/24967/diff/2/?file=683821#file683821line109> > > > > This will make the older clients incompatible with new service. > > We should change the protocol version to V2 so that the older client > > can get a more meaningful error.
Hi Prasad, I updated **SentryPolicyStoreProcessor** and **sentry_policy_service.thrift** to make the API forward compatible, because if the old version client try to connect the new version server, it will be blocked by **privileges is unset** before we compared the version of request in **SentryPolicyStoreProcessor**. In my new patch, I keep the fields **TSentryPrivilege privilege** and ** set< TSentryPrivilege> privileges** and made them **optional**, so it could goto the method in **SentryPolicyStoreProcessor**. Thank you for your review, if you have any comment, feel free to let me know. - Dapeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24967/#review55467 ----------------------------------------------------------- On 十一月 11, 2014, 11:13 p.m., Dapeng Sun wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24967/ > ----------------------------------------------------------- > > (Updated 十一月 11, 2014, 11:13 p.m.) > > > Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > The patch include: > * SENTRY Thrift API changed : > 1. We change the field {{TSentryPrivilege privilege}} to > {{set<TSentryPrivilege> privileges}} in > {{TAlterSentryRoleGrantPrivilegeRequest}} and > {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may > like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, > it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the > request API calls, we make it change. > > 2. Another way to Implement it, maybe add a {{column list}} to > {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore > has many convert methods between {{TSentryPrivilege}} and > {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use > {{TSentryPrivilege}} as query condition, so we should make them one-to-one > correspondence. > > * Change {{SentryStore}} after Thrift API changed > > * Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} > after Thrift API changed, include the grant/revoke methods about column > privilege > > * Change {{Auditlog}} after Thrift API changed > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java > e1d8a9e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java > 2cc8194 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java > 841eeb3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java > 9f16b73 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f6699d2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > 39371b7 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > b20e71e > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > 7e6ade5 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java > cd0a435 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java > fc9c716 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java > 5da8591 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 2e829d6 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 50ee559 > > Diff: https://reviews.apache.org/r/24967/diff/ > > > Testing > ------- > > Unit Tests in local > > > Thanks, > > Dapeng Sun > >
