Linz1248 commented on PR #11972:
URL: https://github.com/apache/seatunnel/pull/11972#issuecomment-5433476955
Thanks @DanielLeens for the thorough review — you're right that `required()`
is presence-only, and removing the `isNotBlank` guards created a blank-value
regression (Issue 1).
I've addressed it by adding `Conditions.notBlank(...)` value constraints on
`client.service-url`, `admin.service-url`, and `subscription.name` in both
factories' `optionRule()`:
- The two URLs are now `.required(option, Conditions.notBlank(option))` —
blank values are caught declaratively at submission time.
- `subscription.name` stays `.optional(option, Conditions.notBlank(option))`
(not promoted to `required`) to avoid the multi-table regression I noted
earlier — the constraint fires only when the value is present but blank.
- Blank-string test cases added for each affected option (5 new tests).
This makes the PR's premise ("covered by OptionRule") actually correct.
Happy to adjust if anything looks off.
@davidzollo — will enable CI workflows on the fork now.
--
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]
