oscerd opened a new pull request, #1812:
URL: https://github.com/apache/camel-kafka-connector/pull/1812
Fixes #1801.
## What
Three behaviour-selecting options are matched with a `switch` whose
`default` branch does nothing, and
none declared a validator, so a typo was accepted at connector submission
and surfaced later as
something unrelated:
| Option | What an unknown value did |
|---|---|
| `camel.idempotency.expression.type` | Left the idempotent expression
unset, so `ckcIdempotent` fell back to its `templateParameter` default
`"dummyExpression"`. `simple()` on a constant means every record after the
first is treated as a duplicate and dropped. |
| `camel.idempotency.repository.type` | Left `idempotentRepo` null, which
was then `bind()`-ed — startup failure with a message unrelated to the actual
typo. |
| `camel.error.handler` | Silently kept a plain `DefaultErrorHandlerBuilder`
**without** the configured `camel.error.handler.max.redeliveries` /
`redelivery.delay`, so the retry policy was ignored. |
Each now declares a `ConfigDef.ValidString`, so Kafka Connect rejects the
value when the configuration
is submitted and names both the option and the allowed values. Applied to
sink and source.
## Compatibility
This is deliberately fail-closed and is a behaviour change: a connector
configuration carrying one of
these typos is accepted today and will be rejected after this change. That
seems like the point — the
first case silently drops every record but the first — but it is worth a
release note, and worth
saying out loud in case you would rather it warned than rejected.
## Tests
`ConfigValidationTest`, 7 cases: each of the three options rejected on sink
and/or source, every
documented value accepted, and the defaults accepted. Confirmed the tests
actually discriminate —
against `main` without the validators, the 5 rejection cases fail with
`Expected org.apache.kafka.common.config.ConfigException to be thrown, but
nothing was thrown`.
## Verification
- `core`: full suite passes (112 tests, +7).
- `./mvnw -Psourcecheck -Dcheckstyle.failOnViolation=true
checkstyle:checkstyle`: BUILD SUCCESS
(the added validator argument pushed four `.define(...)` lines to 251
chars, one over the limit;
they are wrapped).
- Full reactor build from the repository root (`./mvnw clean install
-DskipTests`): BUILD SUCCESS.
## Not done here
The issue also suggests making the `switch` `default` branches throw so the
route templates need no
live fallback defaults. With validation in place those branches are
unreachable through the connector
config, but `CamelKafkaConnectMain.Builder` is still usable directly. Left
alone to keep this change
minimal — happy to follow up if you want it.
--
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]
