Croway commented on PR #2013:
URL:
https://github.com/apache/camel-spring-boot/pull/2013#issuecomment-5935693991
_Claude Code on behalf of Croway_
Thanks for the review. Addressed in the third commit:
1. **`stop()` → `close()`**: added a comment on the concurrent stop: Camel
stops a context under a lock (`BaseService.stop()`), so whichever of the
"Terminate JVM task" and `close()` comes second waits for the first one, then
finds it stopped. Added
`CliConnectorSpringLifecycleTest.camelStopClosesTheApplicationContext`: file
transport, the lock file `~/.camel/{pid}` is deleted (what `camel stop` does),
the context ends up closed.
2. **Upgrade guide**: agreed, it is more of a fix. apache/camel#27210 and
#27214 are merged, so it would be a small docs PR on Camel `main` (TBD).
3. **Condition on an implementation**: right, the Jakarta API alone matched.
Replaced the `@ConditionalOnClass` with `OnSpringWebSocketClientCondition`:
spring-websocket, the Jakarta API, and a `ContainerProvider` service found with
`ServiceLoader` (as `ContainerProvider.getWebSocketContainer()` looks it up,
without creating it). It also does not match with
`camel.cli.websocket.client=jdk`. Tested with a `FilteredClassLoader` hiding
`META-INF/services/jakarta.websocket.ContainerProvider`.
4. **`ssl-bundle` ignored**: a WARN at startup when
`camel.cli.transport=websocket`, `camel.cli.websocket.ssl-bundle` is set and
the JDK client is used (`client=jdk`, or no spring-websocket / Jakarta
implementation). Tested for both cases.
5. **Defaults drift**: added a comment on
`CliConnectorConfiguration.Websocket` pointing at
`WebSocketCliConnectorTransport`.
6. **`Cli Connector` title**: the page is generated
(`update-starter-doc-page`), which takes the title from the catalog for
components, data formats and languages only, and capitalizes the starter name
otherwise. Fixing it means teaching the generator about `other` catalog
entries, which would change other generated pages too, so I left it out of this
PR.
Related to apache/camel#27220 (CAMEL-25230): the Spring client opened
connections on a `SimpleAsyncTaskExecutor`, whose threads are not daemon
threads by default. When the application fails to start, Camel never stops the
connector, which keeps reconnecting, so a connect in progress could delay the
exit. They are daemon threads now (tested), as the threads of the transport.
The upgrade guide note of point 2 could also go next to the one #27220 adds in
the `camel-cli-connector` section of the 4.23 guide.
--
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]
