gnodet commented on PR #26845: URL: https://github.com/apache/camel/pull/26845#issuecomment-5854273536
Addressed all inline findings in cc0bb0fe070c: 1. **Scope** — `onSecretRotation()` now collects DataSources from the component's own field plus all active endpoints belonging to the component (via `getCamelContext().getEndpoints()` filtered by `ep.getComponent() == this`), with identity-based deduplication. This covers `jdbc:myDs`, `sql:...?dataSource=#myDs`, and the autowired single-DataSource case. 2. **Docs/Javadoc** — Removed the incorrect `quarkus.datasource.jdbc.url` reference (that's the URL, not the password). Added explicit note about Quarkus using Agroal (not HikariCP). Removed the contradictory "so that the pool rebuilds them with the rotated credentials" wording — now consistent with the IMPORTANT note throughout. 3. **InvocationTargetException** — Caught separately, logs `e.getCause()` (not the wrapper), and returns immediately instead of falling through to the misleading INFO. 4. **Docs wording** — Rewritten in all four copies (jdbc/sql source + catalog) to accurately describe scope and avoid misleading credential-refresh claims. **On the remaining open points:** - **Tests** — Added endpoint-level DataSource eviction tests for both components. Re: a real `HikariDataSource` + h2 test: agree it would be better coverage, but it would add a test-scope dependency on HikariCP to camel-jdbc (which doesn't have it today) and the test would be integration-ish. Happy to add it if you think it's worth the dependency — the reflection path is the same either way. - **`camel-support` placement** — The helper uses only `javax.sql.DataSource`, `org.apache.camel.Endpoint`, and `org.apache.camel.Component`, all from `camel-api`. It's not Hikari-specific at the API level (the Hikari part is pure reflection with no compile dependency). Moving it to a jdbc-specific module would mean either duplicating it between camel-jdbc and camel-sql, or creating a shared camel-jdbc-support module. `camel-support` seems like the pragmatic home since it's a utility class pattern consistent with other helpers there (`CamelContextHelper`, `PropertyBindingSupport`, etc.). -- 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]
