> On Nov. 1, 2014, 1:45 a.m., Lenni Kuff 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> > > > > should this be an || ? > > Dapeng Sun wrote: > Hi Lenni, Thank you for your comments. In my opinion, status is used for > thrift server, so we should check both status of thrift service and web > service. correct me if I misunderstood.
It should not be ||, as we should not just return if one of them(thrift server and web server) is running. - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27425/#review59449 ----------------------------------------------------------- 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 > >
