> On April 18, 2017, 9:15 p.m., Vamsee Yarlagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 275 (patched) > > <https://reviews.apache.org/r/58221/diff/15/?file=1693584#file1693584line307> > > > > I know it doesn't hurt to check for hmsFollower != null but to make the > > code more readable can we replace this constraint with > > (notificationLogEnabled == true)?
check notification itself is not enough. If hmsFollower is null, we don't want to continue scheduling its task. I was checking both notifcation and hmsFollower. As you mentioned, checking notificationLogEnabled makes code more readable. But Kalyan wants to get rid of it to make the code simplier. So how can I satisfy both preference? Or you two can reach agreement? - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review172262 ----------------------------------------------------------- On April 18, 2017, 3:46 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58221/ > ----------------------------------------------------------- > > (Updated April 18, 2017, 3:46 a.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, kalyan kumar > kalvagadda, and Sergio Pena. > > > Repository: sentry > > > Description > ------- > > SENTRY-1649 move HMS follower to runServer > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 16676fb > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 132db63 > > > Diff: https://reviews.apache.org/r/58221/diff/15/ > > > Testing > ------- > > > Thanks, > > Na Li > >
