CAMEL-24068: camel-spring-rabbitmq - Align component replyTimeout default to 30000 - #24719
Conversation
…ault to 30000 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-scoped fix that corrects a long-standing inconsistency between the component-level and endpoint-level replyTimeout defaults in camel-spring-rabbitmq.
What's happening: CAMEL-19373 (2023) updated the endpoint default to 30000ms but missed the component class, which kept the old 5000ms default. Since createEndpoint() unconditionally copies component.replyTimeout → endpoint.replyTimeout, the effective timeout was always 5s despite the documentation saying 30s. This PR fixes the root cause.
One suggestion:
Per the project's upgrade guide policy (changed defaults must be documented), a brief entry in docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc would help users who were (unknowingly) relying on the effective 5-second timeout:
=== camel-spring-rabbitmq
The `replyTimeout` option on the component level has been fixed to default to 30 seconds,
aligning it with the endpoint-level default that was documented since Camel 3.20.7 / 3.21.
Previously, the component-level default was still 5 seconds, which would override the
endpoint's declared 30-second default. If you were relying on the effective 5-second timeout,
you can restore it by explicitly setting `replyTimeout=5000` on the component or endpoint.That said, this is arguably a bug fix rather than an intentional behavior change — I'll leave it to your judgment whether the upgrade guide entry is warranted here.
Claude Code review on behalf of @gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 12 tested, 26 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
oscerd
left a comment
oscerd
left a comment
There was a problem hiding this comment.
The alignment itself is reasonable — making the component-level replyTimeout default (was 5000 ms) match the endpoint's 30000 ms removes a confusing inconsistency, and the added test pins it.
One thing before merge: this is a user-visible changed default (request/reply producers now wait up to 30s instead of 5s), which per the contributor guidelines should get a short entry in docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc. The PR regenerates the catalog/JSON/DSL correctly but doesn't touch the upgrade guide. A one-liner noting the new default (and that anyone relying on the old 5s behaviour should set replyTimeout=5000 explicitly) would cover it.
Reviewed with Claude Code on behalf of Andrea Cosentino. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
4 participants
Claude Code on behalf of davsclaus
Summary
The
replyTimeoutoption onSpringRabbitMQComponenthad a default of 5000ms (5 seconds), while the correspondingSpringRabbitMQEndpointoption documented and declared a default of 30000ms (30 seconds). SincecreateEndpoint()unconditionally copies the component value to the endpoint, the effective default was always 5 seconds — contradicting the endpoint documentation.This mismatch was introduced in CAMEL-19373 (Jun 2023), which updated the endpoint default from 5000 to 30000 to match Spring's
AsyncRabbitTemplatedefault receive timeout, but did not update the component's default.Changes
SpringRabbitMQComponent.java: ChangedreplyTimeoutfield default from5000to30000, updated@Metadata(defaultValue)annotation and description textSpringRabbitMQComponentTest.java: New unit test verifying the effective endpoint default is 30000ms after creation through the componentcamel-4x-upgrade-guide-4_22.adocdocumenting the changed default (per review feedback from @gnodet and @oscerd)Test plan
SpringRabbitMQComponentTest.testReplyTimeoutDefault()verifies the effective defaultmvn verify— 35 tests, 0 failures)