jinhyukify commented on code in PR #7540:
URL: https://github.com/apache/hbase/pull/7540#discussion_r2649129657
##########
hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java:
##########
@@ -272,16 +284,16 @@ private void process(String urlString) throws Exception {
// disallowed in configuration" in Jetty 9
LogLevelExceptionUtils.validateResponse(connection, 200);
- // read from the servlet
-
try (
InputStreamReader streamReader =
new InputStreamReader(connection.getInputStream(),
StandardCharsets.UTF_8);
BufferedReader bufferedReader = new BufferedReader(streamReader)) {
- bufferedReader.lines().filter(Objects::nonNull).filter(line ->
line.startsWith(MARKER))
- .forEach(line ->
System.out.println(TAG.matcher(line).replaceAll("")));
+
+ return bufferedReader.lines().filter(Objects::nonNull)
+ .filter(line -> line.startsWith(MARKER)).map(line ->
TAG.matcher(line).replaceAll(""))
+ .collect(Collectors.joining("\n"));
} catch (IOException ioe) {
- System.err.println("" + ioe);
+ return "" + ioe;
Review Comment:
Thanks for the review. Good catch.
One thing I wanted to mention is that the previous behavior already felt a
bit inconsistent.
We were catching only `IOException` here and printing it to standard error,
while other exceptions were simply propagated..
That special handling for IOException feels slightly odd to me. Since this
is a CLI command, I am considering not catching `IOException` here at all and
letting it propagate, the same way other exceptions do.
I understand that this could technically be a behavior change for users who
rely on standard error, but printing only this specific exception to stderr
also seems inconsistent from a CLI semantics point of view.
I would like to hear your thoughts on whether there is a strong reason to
keep this special case.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
