gnodet-bot commented on code in PR #28116:
URL: https://github.com/apache/camel/pull/28116#discussion_r4315199396


##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -83,6 +84,11 @@ public final class IntegrationLauncher {
      */
     private static final Map<Path, String> STOPPED_BY_RESTART = new 
ConcurrentHashMap<>();
 
+    /**
+     * The camel command that integrations are started with; a test starts 
another program instead.
+     */
+    static Supplier<List<String>> camelCommand = 
LauncherHelper::getCamelCommand;

Review Comment:
   💡 `camelCommand` is package-private and mutable to allow test injection (a 
test can swap it for a no-op that starts another program instead). Surefire 
runs test classes sequentially within a fork, so there's no race between tests 
mutating this field. Acceptable as-is.



##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -642,6 +652,24 @@ static String restart(long pid) {
         }
     }
 
+    /**
+     * Removes the console output of the process once the process has ended 
(CAMEL-25818). The tools read it only to
+     * answer (the cause of a start that failed, or the output of a run that 
is still starting), and the log of the
+     * integration has what it logged. An integration that outlives this JVM 
keeps it while it runs, as one run with
+     * camel run --background does: camel stop removes it, and camel dirty 
once the process is gone.
+     */
+    static void removeWhenEnded(Process process, Path output) {
+        process.onExit().thenRun(() -> removeConsoleOutput(output));

Review Comment:
   ⚠️ `removeWhenEnded` registers `removeConsoleOutput` via 
`process.onExit().thenRun(...)` with no `.exceptionally()` or `.handle()` 
chained. `removeConsoleOutput` catches `IOException` internally, so 
file-deletion failures are already swallowed silently. The remaining unhandled 
case is `SecurityException`, which is theoretical given `SecurityManager` is 
deprecated and removed in JDK 17+. Not a blocker, but worth keeping in mind if 
this ever runs under a security manager.



-- 
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]

Reply via email to