bejancsaba commented on code in PR #6820:
URL: https://github.com/apache/nifi/pull/6820#discussion_r1060952136
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/service/MiNiFiCommandSender.java:
##########
@@ -61,6 +61,7 @@ public Optional<String> sendCommand(String cmd, Integer port,
String... extraPar
LOGGER.debug("Connecting to MiNiFi instance");
socket.setSoTimeout(SOCKET_TIMEOUT);
socket.connect(new InetSocketAddress("localhost", port),
CONNECTION_TIMEOUT);
+ socket.setSoTimeout(SOCKET_TIMEOUT);
Review Comment:
To be honest I'm not sure. In the old MiNiFi implementation there were both
setting. In the current RunNiFi.sendRequest code the same pattern is followed
similarly to NiFiRegistry. This got removed at a refactor when we moved code
here. However in another place I know @ferencerdei removed the second
setSoTimeout and it caused issues so it was added back. Here I haven't faced
any particular issues explicitly because of this just added it back for the
sake of consistency with the other places in the code. If you feel strongly
about it I can remove of course.
##########
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/bootstrap/BootstrapListener.java:
##########
@@ -291,7 +293,7 @@ private void writeDump(OutputStream out) {
private void echoRequestCmd(String cmd, OutputStream out) {
try {
out.write((cmd + "\n").getBytes(StandardCharsets.UTF_8));
- out.flush();
+ out.close();
Review Comment:
Yeah it is possible I tried to look around and it is basically a no brainer
that close should also do flush and it USUALLY does but it is not GUARANTEED. I
added flush back just to be sure.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
