yigitcan-ozturk commented on issue #11007:
URL: https://github.com/apache/seatunnel/issues/11007#issuecomment-5903338239
Thanks I audited the current `dev` Source path and will keep this to one
focused slice.
Proposed first slice: ClickHouse Source `host` nonblank validation only.
Current `dev` behavior
----------------------
`ClickhouseSourceFactory.optionRule()` currently declares:
.required(HOST, USERNAME, PASSWORD)
so `host` is required for presence, but there is no declarative nonblank
constraint.
`ClickhouseSourceConfig.of(...)` then reads the value directly and
`createSource(...)` passes it into `ClickhouseUtil.createNodes(...)`. I did not
find an earlier Source-side validation that rejects an empty or whitespace-only
`host` before connection/node construction.
I am deliberately not including `batch_size` or `split.size` in this slice.
`ClickhouseTableConfig.of(...)` currently treats values <= 0 as
fallback-to-default:
if (tableConfig.getBatchSize() <= 0) {
tableConfig.setBatchSize(CLICKHOUSE_BATCH_SIZE.defaultValue());
}
if (tableConfig.getSplitSize() <= 0) {
tableConfig.setSplitSize(CLICKHOUSE_SPLIT_SIZE.defaultValue());
}
Turning those into declarative numeric rejection would therefore change
established behavior rather than just move an existing validation boundary.
I am also not including `table_path` / `sql`: their current contract is
relational (`table_path` and `sql` cannot both be empty), and SQL/table-path
selection also affects the existing source strategy, so I would leave that out
of this focused PR.
Proposed declarative constraint
-------------------------------
Source only:
required(HOST, Conditions.notBlank(HOST))
No Sink or ClickhouseFile changes.
Focused ConfigValidator/factory test boundary
---------------------------------------------
For `host`:
- valid nonblank host -> accepted
- missing host -> rejected
- empty host -> rejected
- whitespace-only host -> rejected
- padded nonblank host -> accepted
Existing required-option behavior for `username` and `password` remains
unchanged in this slice.
Explicitly unchanged
--------------------
This PR would not change:
- ClickHouse connection/node construction
- authentication behavior
- username/password semantics
- table_path / sql selection or validation
- table_list behavior
- batch_size or split.size fallback semantics
- partition/filter behavior
- sharding/node selection
- server timezone handling
- clickhouse.config handling
- remote connectivity or metadata checks
- Sink behavior
- ClickhouseFile behavior, including the delimiter work already represented
by #12512
If this Source `host`-only slice matches the intended boundary, I’ll
implement it as one focused PR with the factory/ConfigValidator coverage above.
--
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]