> On Nov. 1, 2014, 1:35 a.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java, > > lines 263-264 > > <https://reviews.apache.org/r/27425/diff/2/?file=746586#file746586line263> > > > > Nit: This is redundant as both the conditions are being checked again? > > Dapeng Sun wrote: > Sravya, Thank you for your time. I think **status** is used for thrift > server, so we should check both status of thrift service and web service. > correct me if I misunderstood.
That is true, but we are again testing for thrift service aliveness on line 267 and web server aliveness on line 279 isnt it? - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27425/#review59448 ----------------------------------------------------------- On Nov. 1, 2014, 1:01 a.m., Dapeng Sun wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27425/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2014, 1:01 a.m.) > > > Review request for sentry, Arun Suresh, Lenni Kuff, Prasad Mujumdar, and > Sravya Tirukkovalur. > > > Bugs: SENTRY-513 > https://issues.apache.org/jira/browse/SENTRY-513 > > > Repository: sentry > > > Description > ------- > > ````java > public synchronized void stop() throws Exception{ > if (status == Status.NOT_STARTED) { > return; > } > LOGGER.info("Attempting to stop..."); > > if (thriftServer.isServing()) { > thriftServer.stop(); // XXX If got exception here, the code after it > will never be executed. > } > thriftServer = null; > stopSentryWebServer(); > status = Status.NOT_STARTED; > LOGGER.info("Stopped..."); > } > ```` > > if {{thriftServer}} got an exception when do stop, the method > {{stopSentryWebServer()}} will be passed > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java > 0243c48 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > ec17480 > > Diff: https://reviews.apache.org/r/27425/diff/ > > > Testing > ------- > > All Unit Tests passed in my jenkins. > > > Thanks, > > Dapeng Sun > >
