Linz1248 opened a new pull request, #11972:
URL: https://github.com/apache/seatunnel/pull/11972
## Purpose of this pull request
Part of #11007.
Cleans up imperative validation in `connector-pulsar` that is already
covered by the declarative `OptionRule` framework (see #10976 / #10977,
following the pattern established in #11095), and adds factory-level validation
tests.
- **Remove redundant presence checks**
- `PulsarConsumerConfig.Builder#build`: the non-blank subscription-name
check is unreachable — every consumer config is built from a
`PulsarTableConfig` produced by `PulsarMultiTableConfig.of`, which already
validates `subscription.name` in `validateTableConfig` (with per-table error
prefixes) before consumer construction.
- `PulsarAdminConfig.Builder#build` / `PulsarClientConfig.Builder#build`:
the non-blank URL checks are already enforced at submission time by
`required(CLIENT_SERVICE_URL, ADMIN_SERVICE_URL)` in the factories' option
rules.
- **Keep runtime-only validation**: the transaction-coordinator check in
`PulsarConfigUtil` depends on client state, not configuration, and stays
imperative per the migration guide.
- **Add factory validation tests**: `ConfigValidator`-based tests in
`PulsarSourceFactoryTest` and `PulsarSinkFactoryTest` cover the required,
exclusive, conditional and bundled option rules.
### Why `subscription.name` is not promoted to `.required(...)`
The option has no default and the consumer requires it, so making it
declaratively required looks correct at first — but multi-table mode allows
`subscription.name` to be overridden per table in `tables_configs`, with
fallback to the global value. Marking the top-level option required would
reject multi-table jobs that only set subscriptions per table.
`PulsarMultiTableConfig.validateTableConfig` already covers both paths (this is
also why the consumer-builder check is redundant rather than merely
duplicated), so the validation stays there.
## Does this PR introduce _any_ user-facing change?
No. The removed checks were redundant: the same conditions are already
enforced earlier by the framework's option-rule validation (service/admin URLs)
or by `PulsarMultiTableConfig.validateTableConfig` (subscription name), so
valid and invalid configurations behave exactly as before.
## How was this patch tested?
Tests were added.
- New `ConfigValidator`-based factory tests cover both positive cases (valid
minimal config; valid config with bundled auth options) and negative cases
(missing client/admin service URL; topic and topic-pattern set together; none
of topic/topic-pattern/table configs set; TIMESTAMP startup mode without
startup timestamp; SUBSCRIPTION startup mode without reset mode; TIMESTAMP stop
mode without stop timestamp; auth plugin class without auth params).
- Local build of the module together with its upstream dependencies: all 60
connector-pulsar tests pass (`Tests run: 60, Failures: 0, Errors: 0, Skipped:
0`).
- `mvn spotless:check` on the module passes.
## Check list
* [ ] If any new Jar binary package is added, add a License Notice per the
[New License Guide](../../../docs/en/developer/new-license.md)
* [ ] If necessary, update the documentation to describe the new feature
* [ ] If necessary, update `incompatible-changes.md` to describe
incompatibility caused by the PR
* [ ] If you contribute connector code, check that the following files are
updated:
1. Update `plugin-mapping.properties` and add new connector information
2. Update the pom file of `seatunnel-dist`
3. Add CI label in `label-scope-conf`
4. Add E2E testcase in `seatunnel-e2e`
5. Update connector `plugin_config`
--
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]
