----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review172014 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java Lines 280 (patched) <https://reviews.apache.org/r/58221/#comment245072> I think it is safer to have both conditions there in case someone else decide to create hmsFollower regardless of the notification enabled or not. It also makes the intention of the code clear: the action should be done only when notification log is enabled. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java Lines 326 (patched) <https://reviews.apache.org/r/58221/#comment245073> It's from c# coding style. I will udpate this. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java Lines 328 (patched) <https://reviews.apache.org/r/58221/#comment245074> The close() method of an AutoCloseable object is called automatically when exiting a try-with-resources block for which the object has been declared in the resource specification header. This construction ensures prompt release, avoiding resource exhaustion exceptions and errors that may otherwise occur. HmsFollower is not created in a try-with-resources block, and we need to call its close() explicitely sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java Lines 333 (patched) <https://reviews.apache.org/r/58221/#comment245075> will do - Na Li On April 14, 2017, 3:41 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58221/ > ----------------------------------------------------------- > > (Updated April 14, 2017, 3:41 p.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/10/ > > > Testing > ------- > > > Thanks, > > Na Li > >
