DanielLeens commented on PR #11788:
URL: https://github.com/apache/seatunnel/pull/11788#issuecomment-5558557774
Thanks @nzw921rx and @davidzollo for taking a look!
One thing worth double-checking before this merges: the head is still
`5a3f7f08` — the same commit I reviewed on 2026-08-13 — and no follow-up commit
has landed since to address the two High-severity findings from that review:
- **Issue 1**: the new DataHub sink FAQ documents a
`${table}`/`${table_name}` multi-table topic-routing placeholder that doesn't
exist in `connector-datahub` — `DataHubSink`/`DataHubWriter` pass the
configured `topic` string straight through with no templating anywhere in the
module (`DataHubWriter.java:82,91`). Following the documented example in a
multi-table job would route every table to one literal (likely nonexistent)
topic name.
- **Issue 2**: the FAQ also claims retry-exhausted DataHub write failures
are "surfaced to the job," but `DataHubWriter.write()` only logs and swallows
`DatahubClientException` (`DataHubWriter.java:90-105`), and the `retry()`
helper has a bug that logs a false "success" after just one iteration whenever
`retryTimes != 0`. So failures are neither surfaced nor accurately logged today.
Both are in `docs/en/connectors/sink/Datahub.md` /
`docs/zh/connectors/sink/Datahub.md` and are unrelated to the other four
connectors' FAQs in this PR, which I verified accurate. Given these describe
user-facing behavior that doesn't match the actual connector, I'd still treat
them as blockers rather than something to fix in a follow-up — want to flag
this before it merges on the strength of the LGTMs 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]
