[Improve][Connector-V2] Migrate MQTT connector validation to declarative OptionRule#11461
[Improve][Connector-V2] Migrate MQTT connector validation to declarative OptionRule#11461ealeonraz wants to merge 1 commit into
Conversation
|
@ealeonraz Thank you for your contribution! This improvement further enhances the configuration verification system of SeaTunnel, allowing configuration issues to be exposed earlier and more clearly, which is very helpful in improving overall usability and maintainability.🚀 pls enable CI: https://github.com/apache/seatunnel/pull/11461/checks?check_run_id=87482860873 |
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I rechecked the latest head cee75286b7a6 against the normal MQTT source/sink creation path, and I do not see a source-level blocker on the current diff.
The validation move looks consistent to me:
- source path:
MqttSourceFactory.optionRule() -> MqttSource(ReadonlyConfig) -> MqttSourceConfig - sink path:
MqttSinkFactory.optionRule() -> MqttSink.createWriter(...) -> MqttSinkWriter - the new factory tests cover the moved constraints (
qos,format,reconnect_timeout,max_queue_size,batch_size, and the persistent-sessionclient_idrequirement), so this is not just deleting checks without replacement
The remaining gate is CI rather than source logic. Right now Build is ACTION_REQUIRED because workflow run detection failed, and the check summary is explicitly asking for two things:
- enable GitHub Actions in the fork
- trigger a fresh run
This branch is also behind the latest dev by 1 commit (ahead_by=1, behind_by=1, compare status diverged), so the lowest-cost next step is:
- sync with the latest
dev - enable Actions in the fork if they are still disabled there
- push an empty commit or otherwise retrigger CI
If the refreshed run still fails after that, please share the new check link here and I can help narrow it down further.
7f0f6ac to
8175004
Compare
|
@DanielLeens Done on both counts: Actions enabled in the fork, and the branch is rebased onto the latest Refreshed run: https://github.com/ealeonraz/seatunnel/actions/runs/29519036639 Everything is green except the Evidence it's independent of this change:
I didn't touch the engine test in this PR to keep the scope to the connector-mqtt slice, but I'm happy to file a separate issue for |
Purpose of this pull request
Part of #11007 (connector-mqtt slice). Migrates the MQTT connector's option validation from imperative
if/throwchecks to declarativeoptionRule()+Conditions.*, so invalid configs are caught at--check/submission time and constraints are exposed via option-rule metadata.Migrated rules:
MqttSourceFactory):qosmust be 0 or 1,formatmust bejson/text(case-insensitive, via a sharedMqttFormatValidatorConditionExtension),reconnect_timeout> 0,max_queue_size> 0, andclient_idrequired and non-blank whenclean_session=false. The client_id rule is expressed as a conditional requirement plus a conditionalnotBlankvalue constraint, because conditional value constraints alone are skipped when the option is absent — a single rule would silently acceptclean_session=falsewithout aclient_id.MqttSinkFactory):qosmust be 0 or 1,formatmust bejson/text,batch_size>= 1.Kept in runtime code (per the migration guide's "not suitable for declarative rules" section): broker connection error handling and the serialization-format dispatch
defaultbranch. No options were renamed and no defaults changed.Does this PR introduce any user-facing change?
No config option changes. Invalid option values are now rejected during config validation (
OptionValidationExceptionat--check/submission time) instead ofIllegalArgumentExceptionat connector initialization.How was this patch tested?
Existing imperative-validation unit tests were migrated to
ConfigValidator-based tests against the factory option rules, plus new boundary cases (qos 0/1 accepted, batch_size 1 accepted, case-insensitive formats, and all three client_id scenarios: absent fails, blank fails, present passes)../mvnw spotless:applyand./mvnw -DskipTests verifyrun clean on the module.Check list
incompatible-changes.mdnot affected)