dajac commented on a change in pull request #9334:
URL: https://github.com/apache/kafka/pull/9334#discussion_r494815168
##########
File path: core/src/main/scala/kafka/admin/TopicCommand.scala
##########
@@ -69,16 +71,26 @@ object TopicCommand extends Logging {
else if (opts.hasDeleteOption)
topicService.deleteTopic(opts)
} catch {
+ case e: ExecutionException =>
+ if (e.getCause != null)
+ printException(e.getCause)
+ else
+ printException(e)
+ exitCode = 1
case e: Throwable =>
- println("Error while executing topic command : " + e.getMessage)
- error(Utils.stackTrace(e))
+ printException(e)
exitCode = 1
} finally {
topicService.close()
Exit.exit(exitCode)
}
}
+ private def printException(e: Throwable): Unit = {
+ println("Error while executing topic command : " + e.getMessage)
+ error(Utils.stackTrace(e))
Review comment:
Do you mean using `error(message, e)` to replace both `println` and
`error`? I think that we are using 'println` here in order to print the message
to stdout without any logger related stuff and regardless of how the logger is
configured. Changing to using `error(message, e)` would break this and
potentially break existing application due to introducing the logger related
stuff for that message. I think that we should keep `println` here.
However, I wonder if using `error` is appropriate here as it basically
prints a stacktrace for every errors. The UX does not look good. Would it make
sense to use `debug` instead? The message of the exception is printed anyway
and I don't think that the stacktrace provides much to regular users. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
