gnodet-bot commented on code in PR #27516:
URL: https://github.com/apache/camel/pull/27516#discussion_r4212371686
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/LaunchManager.java:
##########
@@ -358,38 +418,68 @@ private void checkDeferredLaunch(long now) {
DeferredLaunch dl = deferredLaunch;
deferredLaunch = null;
dl.launchAction.run();
- } else if (now - deferredLaunch.startTime > 120_000) {
+ } else if (!infraStarting(deferredLaunch.requiredInfra)) {
+ // the infra failed to start: the failure log says why
+ DeferredLaunch dl = deferredLaunch;
+ deferredLaunch = null;
+ notify("Not started: " + dl.displayName() + " (its infra
services failed to start)", true);
+ } else if (now - deferredLaunch.startTime > INFRA_WATCH_MS) {
+ // a first start pulls the container image, which can take
minutes: wait while the infra is starting
deferredLaunch = null;
notify("Timeout waiting for infra services to start", true);
}
}
}
private void monitorPendingLaunches(long now) {
+ Set<String> runningAliases = null;
Iterator<PendingLaunch> it = pendingLaunches.iterator();
while (it.hasNext()) {
PendingLaunch pl = it.next();
+ if (pl.infraAlias != null && !pl.started) {
+ if (runningAliases == null) {
+ runningAliases = infraServices.get().stream()
+ .filter(i -> i.alive).map(i ->
i.alias).collect(Collectors.toSet());
+ }
+ // an infra service has started when it is up
+ if (runningAliases.contains(pl.infraAlias)) {
+ pl.started = true;
+ notify("Started: " + pl.name, false);
+ pl.announced = true;
+ }
+ }
if (!pl.process.isAlive()) {
int exitCode = pl.process.exitValue();
- if (exitCode == 0 || pl.started) {
+ boolean ok = pl.infraAlias != null ? pl.started : exitCode ==
0 || pl.started;
+ if (ok) {
if (!pl.announced) {
notify("Started: " + pl.name, false);
}
- } else if (failureLogCallback != null) {
- failureLogCallback.accept(pl.name, pl.outputFile);
+ outcomes.put(pl.name, LaunchOutcome.started());
+ } else {
+ outcomes.put(pl.name, LaunchOutcome.failed(pl.outputFile));
+ if (failureLogCallback != null) {
+ failureLogCallback.accept(pl.name, pl.outputFile);
+ }
}
it.remove();
} else if (pl.started) {
// up and running: a stop or a failure from now on is not a
failed start
+ outcomes.put(pl.name, LaunchOutcome.started());
it.remove();
} else if (pl.startFailed()) {
// the app gave up starting (port in use, build failure) but
its JVM lives on: stop it, and show why
pl.process.descendants().forEach(ProcessHandle::destroy);
pl.process.destroy();
+ outcomes.put(pl.name, LaunchOutcome.failed(pl.outputFile));
if (failureLogCallback != null) {
failureLogCallback.accept(pl.name, pl.outputFile);
}
it.remove();
+ } else if (pl.infraAlias != null) {
+ if (now - pl.startTime > INFRA_WATCH_MS) {
+ it.remove();
Review Comment:
💡 **Minor:** Every other removal path records an outcome — process-not-alive
records `LaunchOutcome.started()` or `LaunchOutcome.failed()`, `pl.started`
records `started()`, `pl.startFailed()` records `failed()` — but this
infra-timeout branch just removes silently. The missing entry is harmless for
current callers (the MCP poll loop doesn't observe it), but it breaks the
invariant that every removed `PendingLaunch` has a corresponding outcome, which
could confuse future consumers of `outcomes`.
Consider adding:
```suggestion
outcomes.put(pl.name,
LaunchOutcome.failed(pl.outputFile));
it.remove();
```
--
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]
