----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54454/#review158278 -----------------------------------------------------------
sentry-service/sentry-service-common/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java (line 16) <https://reviews.apache.org/r/54454/#comment229050> This file is auto-generated by Thrift, I don't think we should touch these. sentry-service/sentry-service-common/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java (line 981) <https://reviews.apache.org/r/54454/#comment229051> This is also auto-generated file sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1431) <https://reviews.apache.org/r/54454/#comment229052> Why do you want to leave this line here? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1432) <https://reviews.apache.org/r/54454/#comment229053> Why privilege.setActionIsSet(false) is called here? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1453) <https://reviews.apache.org/r/54454/#comment229054> Why do you want to leave the old code behind? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1720) <https://reviews.apache.org/r/54454/#comment229056> MSentryPrivilege is generated by Sentry code itself so it is weird to complain about invalid privilege here. I guess the check should be for the original privilege instead. And the message probably should be something like "grant option is not specified" - Alexander Kolbasov On Dec. 6, 2016, 11:05 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54454/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2016, 11:05 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, > and Vadim Spector. > > > Repository: sentry > > > Description > ------- > > SENTRY-1548 Setting GrantOption to UNSET upsets Sentry > > I have made changes assuming that grant option is either true/false removing > unset. > Also, added code so that sentry server could validate the TSentryPrivilege > object constructed from the Thrift message received client. If the validation > is failed exception is raised and appropriate error is message is sent. > > > Diffs > ----- > > > sentry-service/sentry-service-common/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java > c056bcc > > sentry-service/sentry-service-common/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java > 15b339f > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 742798d > > Diff: https://reviews.apache.org/r/54454/diff/ > > > Testing > ------- > > Verfied the changes using sentry thrift client. > > > Thanks, > > kalyan kumar kalvagadda > >
