gnodet commented on PR #28147: URL: https://github.com/apache/camel/pull/28147#issuecomment-6316060692
Thanks for the thorough review @oscerd! The second commit (4ac39e99) addresses all the actionable findings — it was pushed just as the review came in: **Blocker — isRunAllowed() gate:** Replaced `getCamelContext().isStarted()` with `isRunAllowed()` as suggested. This correctly handles the RecipientListProcessor case where wrappers are built at runtime during the Starting and Stopping phases. **Finding 3 — errorHandlers lifecycle:** Changed `doStop()` and `doShutdown()` to pass `errorHandlers.values()` instead of the map itself, making the lifecycle symmetric. **Finding 2 — test coverage:** The test now sends a message after the route reaches `Started` and verifies the process count. `ServiceStatus.Started` is used for comparison (not toString). **Finding 1 — upgrade guide:** Added to `camel-4x-upgrade-guide-4_24.adoc`. **Finding 4 — Splitter watermark (pre-existing):** Agreed, this is a separate concern. I'll open a dedicated JIRA. **Finding 5 — PR title:** The title already says "Start Split and Multicast output producers when the route starts" which describes the fix. Happy to adjust if there is a preferred wording. CI is running on the updated commit. -- 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]
