Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

2016-12-08 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54464/#review158619 --- Ship it! Ship It! - Alexander Kolbasov On Dec. 9, 2016, 1:16

Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

2016-12-08 Thread Vamsee Yarlagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54464/ --- (Updated Dec. 9, 2016, 1:16 a.m.) Review request for sentry, Alexander Kolbasov

Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

2016-12-08 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54464/#review158589 --- The fix is good, a few suggestions for the test. sentry-service/

Re: Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

2016-12-08 Thread Alexander Kolbasov
> On Dec. 8, 2016, 9:32 p.m., Alexander Kolbasov wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 1607 > > > > > > So this is

Re: Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

2016-12-08 Thread Vamsee Yarlagadda
> On Dec. 8, 2016, 9:32 p.m., Alexander Kolbasov wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 1607 > > > > > > So this is

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-08 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54525/#review158578 --- I mean convertToTSentryPrivilege() is similar. - Alexander Kolbas

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-08 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54525/#review158576 --- There is also toSentryPrivilege() with similar problem. And once t

Re: Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

2016-12-08 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54526/#review158572 --- sentry-service/sentry-service-server/src/main/java/org/apache/sen

Re: Review Request 54532: SENTRY-378: Changed usages of SentryAccessDeniedException to SentryConfigurationException when retrieving Sentry version information in SentryStore

2016-12-08 Thread Jan Hentschel
> On Dec. 8, 2016, 6:55 p.m., Alexander Kolbasov wrote: > > Are there any other cases that are worth fixing? I haven't seen any other place where SentryAccessDeniedException is used in another way than described in the ticket. Can't speak for the rest of the exceptions. - Jan -

Re: Review Request 54532: SENTRY-378: Changed usages of SentryAccessDeniedException to SentryConfigurationException when retrieving Sentry version information in SentryStore

2016-12-08 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54532/#review158546 --- Ship it! Are there any other cases that are worth fixing? - Al

Re: Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

2016-12-08 Thread kalyan kumar kalvagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54526/#review158542 --- >From what I understand from SENTRY-162, is that dropPrivilege sho

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-08 Thread kalyan kumar kalvagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54525/#review158541 --- Ship it! Looks good - kalyan kumar kalvagadda On Dec. 8, 201

Review Request 54532: SENTRY-378: Changed usages of SentryAccessDeniedException to SentryConfigurationException when retrieving Sentry version information in SentryStore

2016-12-08 Thread Jan Hentschel
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54532/ --- Review request for sentry. Bugs: SENTRY-378 https://issues.apache.org/jira/

Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

2016-12-08 Thread Vamsee Yarlagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54526/ --- Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvaga

Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-08 Thread Vamsee Yarlagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54525/ --- Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvaga