Repository navigation
Conversation
…ever refreshes its cache from a quick command The check is marked before the fetch, the launcher waits for the fetch after the command (at most 2 seconds, so once a day), and the HTTP client of the fetch is closed, as its selector thread delays the exit of the JVM. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Claus Ibsen <[email protected]>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Signed-off-by: Claus Ibsen <[email protected]> # Conflicts: # dsl/camel-jbang/camel-launcher/src/main/java/org/apache/camel/dsl/jbang/launcher/CamelLauncherMain.java
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 1 of 713 tested, 0 compile-only — current: 1 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 1). Skip-tests mode would test 1 modules (1 direct + 1 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (1)
💡 Manual integration tests recommended:
All tested modules (2 modules, 58.0s total)Total reactor time: 58.0s
Top 20 slowest modules:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
LGTM. The concurrency model is correct (happens-before from Thread.start() guarantees the Properties object is safely published to the daemon thread), the failure semantics match the old code (a failed fetch preserves last_checked from markChecked, preventing retry storms), and the integration test correctly validates the fix by running real child JVMs.
One nit: the catch block in refreshCache drops the explanatory comment that was there before.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Claus Ibsen <[email protected]>
gnodet-bot
left a comment
There was a problem hiding this comment.
Previous finding (missing catch-block comment) — ✅ addressed in 0029418. No new issues found.
The concurrency model is correct: Thread.start() provides the happens-before edge that safely publishes the Properties object to the daemon thread, volatile on refresh is appropriate, and the interrupt restore in awaitRefresh() follows the right pattern. The postExecute lifecycle hook correctly orchestrates the wait. The integration test runs each assertion in a fresh JVM, which is the right approach for a component that relies on JVM-scoped static state.
LGTM.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
CAMEL-26443
The launcher of the web installer checks once a day for a newer release: with a cache older than 24 hours, a daemon thread fetches
latest.propertiesand then writesupdate-check.properties. Since the startup work of CAMEL-23810, a quick command (camel version,camel ps, about 170 ms) ends before that thread has even sent its request. The JVM stops it and nothing is written, so a user who only runs quick commands never sees the notice of a new release, and every quick command starts the fetch again. Only a long command (camel run,camel tui) refreshed the cache.Changes
UpdateCheckermarks the check (last_checked, and the target) before the fetch, so a failed or slow fetch is not tried again by the next commands, but the next day. A failed fetch keeps the version of the check before, as it did.UpdateChecker.awaitRefresh(), called byCamelLauncherMain.postExecute, waits after the command for the fetch that this process started, at most 2 seconds. It waits only on the run that started the fetch, so once a day; after a long command the fetch is already done.ManifestFetcherisAutoCloseableand closes itsHttpClient, which the check does after the fetch: the selector thread of the client delays the exit of the JVM by about 300 ms (as in CAMEL-26427: camel-jbang - Every CLI command waits 300 ms before the JVM exits #28762).Measured
camel versionof a web installer install, JDK 25, median of 7, against a manifest server of the test that answers in 50 ms:Tests
UpdateCheckRefreshTestrunscamel versionof a web installer install in a new JVM against anHttpServerof the test: a stale cache is refreshed by the command (one request,latest_versionfrom the server); after a failed fetch (HTTP 500) the next command does not fetch again and the version of the check before is kept. Without the wait inpostExecuteboth fail with no request at all.UpdateCheckerTest,SelfUpdateIntegrationTest,LauncherStartupLoggingTest, ...).Claude Code on behalf of davsclaus (Claus Ibsen)
🤖 Generated with Claude Code
https://claude.ai/code/session_0128ThHB3iQdrvSRLVBGRrdP