gnodet-bot commented on code in PR #26821:
URL: https://github.com/apache/camel/pull/26821#discussion_r4090685582
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -13,6 +13,28 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.22 to 4.23
+=== camel-core - Exchange Pooling deprecated
+
+Exchange pooling (`exchangeFactory=pooled`) is deprecated and will be removed
in a future release.
+The following classes are deprecated:
+
+- `org.apache.camel.PooledExchange`
+- `org.apache.camel.support.DefaultPooledExchange`
+- `org.apache.camel.impl.engine.PooledExchangeFactory`
+- `org.apache.camel.impl.engine.PooledProcessorExchangeFactory`
+- `org.apache.camel.processor.PooledExchangeTaskFactory`
+- `org.apache.camel.processor.PooledTaskFactory`
Review Comment:
⚠️ `CamelInternalPooledTaskFactory` is deprecated in the code (listed in the
PR body) but still absent from this class list. Add it after
`PooledTaskFactory`:
```suggestion
- `org.apache.camel.processor.PooledTaskFactory`
- `org.apache.camel.impl.engine.CamelInternalPooledTaskFactory`
```
##########
core/camel-main/src/main/java/org/apache/camel/main/DefaultConfigurationConfigurer.java:
##########
@@ -166,6 +166,9 @@ public static void configure(CamelContext camelContext,
DefaultConfigurationProp
beanIntrospection.afterPropertiesConfigured(camelContext);
if ("pooled".equals(config.getExchangeFactory())) {
+ LOG.warn(
+ "Exchange pooling (exchangeFactory=pooled) is deprecated
and will be removed in a future release."
+ + " Please remove the
'camel.main.exchange-factory=pooled' configuration.");
Review Comment:
🔧 String concatenation with `+` across two literals. Merge into a single
string:
```suggestion
LOG.warn("Exchange pooling (exchangeFactory=pooled) is
deprecated and will be removed in a future release. Please remove the
'camel.main.exchange-factory=pooled' configuration.");
```
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -13,6 +13,28 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.22 to 4.23
+=== camel-core - Exchange Pooling deprecated
+
+Exchange pooling (`exchangeFactory=pooled`) is deprecated and will be removed
in a future release.
+The following classes are deprecated:
+
+- `org.apache.camel.PooledExchange`
+- `org.apache.camel.support.DefaultPooledExchange`
+- `org.apache.camel.impl.engine.PooledExchangeFactory`
+- `org.apache.camel.impl.engine.PooledProcessorExchangeFactory`
+- `org.apache.camel.processor.PooledExchangeTaskFactory`
+- `org.apache.camel.processor.PooledTaskFactory`
Review Comment:
⚠️ `CamelInternalPooledTaskFactory` is deprecated in the code (PR body lists
it, `@Deprecated(since="4.23.0")` is on the class) but is still missing from
this list. Add it to keep the guide consistent:
```suggestion
- `org.apache.camel.processor.PooledTaskFactory`
- `org.apache.camel.impl.engine.CamelInternalPooledTaskFactory`
```
##########
core/camel-main/src/main/java/org/apache/camel/main/DefaultConfigurationConfigurer.java:
##########
@@ -166,6 +166,9 @@ public static void configure(CamelContext camelContext,
DefaultConfigurationProp
beanIntrospection.afterPropertiesConfigured(camelContext);
if ("pooled".equals(config.getExchangeFactory())) {
+ LOG.warn(
+ "Exchange pooling (exchangeFactory=pooled) is deprecated
and will be removed in a future release."
+ + " Please remove the
'camel.main.exchange-factory=pooled' configuration.");
Review Comment:
💡 The warning message is split across two string literals joined with `+`.
The javac constant-folder collapses it at compile time but the style is noisy.
Collapse into a single text block:
```suggestion
LOG.warn(
"Exchange pooling (exchangeFactory=pooled) is deprecated
and will be removed in a future release."
+ " Please remove the
'camel.main.exchange-factory=pooled' configuration.");
```
(Or simply join the two halves into one literal — no functional difference,
purely style.)
##########
core/camel-api/src/main/java/org/apache/camel/PooledExchange.java:
##########
@@ -23,8 +23,10 @@
* internally by Camel for optimizing memory footprint by reusing exchanges
created by {@link Consumer}s via
* {@link ExchangeFactory}.
*
Review Comment:
📝 **Incomplete deprecation scope — `PooledExchangeTask` and
`ExchangeFactoryManager` not deprecated**
The PR deprecates the concrete pooled implementations and the config layer,
but two types that are exclusively tied to exchange pooling are left without
deprecation markers:
- `org.apache.camel.processor.PooledExchangeTask` — the task interface
implemented only by pooled EIP tasks
- `org.apache.camel.spi.ExchangeFactoryManager` — manages pooled exchange
factories; has no meaning in prototype mode
(`ExchangeFactory`, `ProcessorExchangeFactory`, `PooledObjectFactory`, and
`PooledObjectFactorySupport` are intentionally excluded from this note — they
remain in use via the prototype/default path and should not be deprecated.)
Users with custom `PooledExchangeTask` implementations or direct
`ExchangeFactoryManager` references won't get a compile-time deprecation
warning, making the story incomplete. Both classes should be deprecated in this
PR alongside the others.
--
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]
