Fix camel-jbang logging file appender and jbang java version option#22337
Fix camel-jbang logging file appender and jbang java version option#22337
Conversation
Two fixes for the Camel JBang IT test failures: 1. Add missing file appender to log4j2-no-color.properties. The no-color logging config lacked the file appender that log4j2.properties has. Since CAMEL-23226 changed loggingColor to auto-detect based on environment (EnvironmentHelper.isColorEnabled()), non-TTY environments like Docker containers now use the no-color config, which wrote logs only to console and never to ~/.camel/<pid>.log. This caused camel log to always return empty output, failing 38+ IT tests with timeouts. 2. Fix jbang --java-version option: use --java instead. The --java-version flag is not recognized by jbang. The correct flag to set the Java version is --java. This caused failures when running with --camel-version pointing to older Camel versions.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
All tested modules (26 modules)
|
|
|
||
| if (javaVersion != null) { | ||
| jbangArgs.add("--java-version=" + javaVersion); | ||
| jbangArgs.add("--java=" + javaVersion); |
There was a problem hiding this comment.
should we change the --java-version parameter too which was added in the breaking commit in order to keep consistency between commands? c00319b
There was a problem hiding this comment.
good point, in this case, --java is the JBang one, but I guess I can add an alias (for backward compatibility) to the Camel JBang Run and Export, wdyt?
There was a problem hiding this comment.
I think it should work but worth to be tried, including when using "cross-versions" of Camel and Camel Jbang (which can be potentially used in from command-line and from VS Code integrations) because it reminds me simlar issues than afb0b1a when there was a renaming of property
There was a problem hiding this comment.
good point, I just pushed a new commit that handle cross-version compatibility.. I actually never thought about cross-versions compatibility... well.. it is scary :D but I do think that most of the user stick on the latest Camel JBang version
cad250e to
62dc355
Compare
Keep --java-version for backward compatibility and add --java as an alias in Run, ExportBaseCommand, and KubernetesRun commands. Strip --java/--java-version from cmds in runCamelVersion() to avoid passing an unrecognized option to older Camel versions that only know --java-version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
62dc355 to
7362280
Compare
gnodet
left a comment
There was a problem hiding this comment.
Good fix for two clear regressions. Root cause analysis in the PR description is excellent.
A few observations:
Generated files need regeneration — The picocli annotation changes (adding --java alias) need the generated metadata and docs updated. Currently camel-jbang-commands-metadata.json still shows only --java-version for the export/sbom commands, and the generated docs in docs/user-manual/modules/ROOT/pages/jbang-commands/ also only reference --java-version. The CI "uncommitted changes" check will likely flag this. Run:
mvn install -B -pl dsl/camel-jbang/camel-jbang-core -DskipTestsand commit the regenerated files.
Pre-existing: color config's file appender uses ANSI patterns — Not introduced by this PR, but worth noting: log4j2.properties writes %style{...} / %highlight{...} patterns to the file appender, so ~/.camel/{pid}.log ends up with ANSI escape codes. The no-color fix correctly uses plain patterns for the file, which is actually better behavior.
Claude Code on behalf of Guillaume Nodet
Summary
log4j2-no-color.properties— Since CAMEL-23226 changedloggingColorto auto-detect viaEnvironmentHelper.isColorEnabled(), non-TTY environments (Docker containers, CI) use the no-color log config, which lacked the file appender. Logs went only to console, never to~/.camel/<pid>.log, causingcamel logto return empty and 38+ IT tests to timeout after 5 minutes.--java-version→--java— jbang does not recognize--java-version; the correct flag is--java. This caused 3 IT tests to fail when running with--camel-versionpointing to older Camel versions.Root cause analysis
Timeout failures (38 tests): Regression introduced by
3adff4a7fd4(CAMEL-23226) which changedloggingColordefault fromtruetoEnvironmentHelper.isColorEnabled(). In Docker containers without TTY, this returnsfalse, selectinglog4j2-no-color.propertieswhich had no file appender — unlikelog4j2.propertieswhich logs to both console and file.--java-version failures (3 tests): Bug in
c00319b34c6(CAMEL-23235) which passed--java-version=21to jbang, but jbang's actual option is--java.Test plan
RunCommandITCase#initAndRunTestlocally — was timing out, now passes in ~10sRunCommandITCase#runSpecificVersionTestlocally — was failing withUnknown option, now passesVersionCommandITCase#executeAfterVersionSetTestlocally — was failing, now passes🤖 Generated with Claude Code