----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59510/#review176222 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2890 (patched) <https://reviews.apache.org/r/59510/#comment249563> The name of the method does not match with the result. It says 'getFirst..' but the caller might expect more than 1 result due to the list? We should only return the one object instead. Notice that the object might be null. Another idea is just return True or False if at least 1 row is found. Do you expect that there are other callers which require only one row of the table? If not, why not keeping this block of code inside isAuthzPathsMappingEmpty()? It is only a 2-3 lines of code. - Sergio Pena On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59510/ > ----------------------------------------------------------- > > (Updated May 25, 2017, 1:41 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1774 > https://issues.apache.org/jira/browse/SENTRY-1774 > > > Repository: sentry > > > Description > ------- > > HMSFollower should always depend on persisted information to decide is full > snapshot is needed instead of depending on in-memory information. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 8450402 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 58e8881 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 440b0e9 > > > Diff: https://reviews.apache.org/r/59510/diff/2/ > > > Testing > ------- > > Added new test to check new functionality added and also ran HMSFollower tests > > > Thanks, > > kalyan kumar kalvagadda > >
