> On May 8, 2017, 6:35 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Lines 381 (patched) > > <https://reviews.apache.org/r/59058/diff/1/?file=1710733#file1710733line381> > > > > Why should be throw an exception and then catch it? Why not just avoid > > raising an exception and log the error?
calls to SentryStore in ProcessSingleNotificationEvent could throw exception. So we have to catch exception here. Throwing exception in ProcessSingleNotificationEvent and catching it in its caller is easy to read. We can change those places to log error and call stack and return, but it is not consistent to other code that throws exception in error condition. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59058/#review174210 ----------------------------------------------------------- On May 8, 2017, 6:07 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59058/ > ----------------------------------------------------------- > > (Updated May 8, 2017, 6:07 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, kalyan kumar > kalvagadda, Sergio Pena, and Vamsee Yarlagadda. > > > Bugs: sentry-1752 > https://issues.apache.org/jira/browse/sentry-1752 > > > Repository: sentry > > > Description > ------- > > Catch exception in processing notification event and move on. Add unit test > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > e271ea4 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java > fd97936 > > > Diff: https://reviews.apache.org/r/59058/diff/1/ > > > Testing > ------- > > add unit test > > > Thanks, > > Na Li > >
