> On Dec. 7, 2016, 12:56 a.m., Alexander Kolbasov wrote: > > 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/diff/1/?file=1578186#file1578186line1431> > > > > Why do you want to leave this line here?
I will remove the line > On Dec. 7, 2016, 12:56 a.m., Alexander Kolbasov wrote: > > 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/diff/1/?file=1578186#file1578186line1432> > > > > Why privilege.setActionIsSet(false) is called here? It was a type. It should have been privilege.setGrantOptionIsSet(false) > On Dec. 7, 2016, 12:56 a.m., Alexander Kolbasov wrote: > > 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/diff/1/?file=1578186#file1578186line1453> > > > > Why do you want to leave the old code behind? There is no reason. Just thought of to removing it after the review is done. > On Dec. 7, 2016, 12:56 a.m., Alexander Kolbasov wrote: > > 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/diff/1/?file=1578184#file1578184line16> > > > > This file is auto-generated by Thrift, I don't think we should touch > > these. Sasha, Are you sure that all of the file is auto generated? I thought creates a template file and let lets update it. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54454/#review158278 ----------------------------------------------------------- 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 > >
