rdhabalia commented on a change in pull request #2214: Add tls support to
authenticate client to access function admin-api
URL: https://github.com/apache/incubator-pulsar/pull/2214#discussion_r207388606
##########
File path:
pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/WorkerServer.java
##########
@@ -96,7 +136,6 @@ public void run() {
log.error("ex: {}", ex, ex);
final String message = getErrorMessage(server,
this.workerConfig.getWorkerPort(), ex);
log.error(message);
- System.exit(1);
Review comment:
@sijie integration test is failing because I have removed it in this PR. in
order to test tls, we need to start worker-server and while stopping it in
unit-test, it was throwing interruption exception on the thread which was
causing system.exit in junit process.
and it's also bad practice to do system.exit on exception.
However, this change was causing integration test failure.
to fix it, we can either keep system.exit for now and disable tls-unit test
or we have to fix integration test.
I wanted to refactor this method lil bit to fix it but we merge it before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
