ss666 commented on PR #11544: URL: https://github.com/apache/seatunnel/pull/11544#issuecomment-5112575885
@SEZ9 Thanks for the detailed re-review. Replies inline: **Issue 1 — not an API change.** `FlussSinkIT` now extends `FlussTestBase` instead of duplicating container setup. The public/protected members the finding refers to were moved into the new shared base. `FlussSinkIT` still inherits all of them; the helpers' visibility was narrowed from `public` to `protected` within the test hierarchy (test-only methods on an IT class). There's no downstream compatibility surface. **Issue 2 — both already handled.** - **Lower bound:** `FlussSourceFactory.optionRule()` rejects `<= 0` at submission time via `greaterThan(POLL_TIMEOUT_MS, 0L)` (`FlussSourceFactory.java:53-55`). - **Lock:** I've reworded the "honor the checkpoint lock" line in the PR description that likely prompted this — the reader doesn't take the lock itself; the base reader takes it only around record emission, and `poll()` happens off the lock on the fetcher thread. **Issue 3 — no contradiction; my PR description was wrong.** The code does not implement `SupportColumnProjection` anywhere, and the docs correctly mark column projection as unsupported. The `SupportColumnProjection` mention came from a stale line in my PR description, which I've fixed — the code and docs already agree. **Issue 4 — no live path; keeping for consistency.** **Issue 5 — a style choice with no documented rule; keeping current keys.** As far as I could find, SeaTunnel doesn't document a naming-separator convention. `poll.timeout.ms` is actually consistent with the connector's existing dotted keys, `bootstrap.servers` and `client.config`, which come from `FlussBaseOptions`. So renaming `poll.timeout.ms` to snake_case for consistency would mean renaming those too — and they're shared with the Fluss sink already merged in #10121. -- 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]
