TianHengZhuang commented on PR #12384: URL: https://github.com/apache/seatunnel/pull/12384#issuecomment-5863044668
yo @DanielLeens, all four points are pushed on `fd3d3af` now - thanks for the nudge, you were right that nothing had actually landed on the branch before. here's what i did: 1. **`write_timeout`** - dropped the constraint entirely. you're right that it's declared but never read at runtime, so constraining it was pure downside. the option itself stays declared, just unconstrained now. 2. **`batch_size`** - went with Option A: kept `batch_size > 0` and documented it, since `batch_size = 0` had a real meaning (flush only on checkpoint/close) and silently dropping that is worse than failing fast. added an `incompatible-changes.md` entry (en + zh) describing the change and the migration path. 3. **docs** - added the valid ranges to the en/zh sink docs: `url` / `database` required + not blank, `connect_timeout_ms` / `query_timeout_sec` > 0, `batch_size` > 0. 4. **tests** - extended `InfluxDBFactoryTest`: negative values for the three constrained numeric options, the both-missing url+database case (asserting both keys show up in the message), plus pins for the bundled `username`/`password` and `multi_table_sink_replica` options. PR is now 6 files (+185/-15). Build should kick off on the new head - will keep an eye on it. cheers! -- 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]
