88fantasy commented on PR #12208: URL: https://github.com/apache/seatunnel/pull/12208#issuecomment-5595288182
Thanks for the detailed review — really appreciated the depth here, especially the exception-safety and CI-log tracing. Both blockers are fixed in `1f2c9928f`: **Issue 1** (`LineageRuntime.emit()` rethrowing) — `LineageRuntime.emit()` now catches `Throwable` from backend resolution/emission and logs it at WARNING via `java.util.logging` (kept off `slf4j` deliberately, since this module still carries zero runtime dependencies so it can be shaded into a Flink `lib/` directory). `LineageReportingException` is removed since nothing constructs it anymore. Added `LineageRuntimeTest` covering both the disabled no-op path and the "unresolvable transport degrades to a logged warning, not a throw" case you called out. **Issue 2** (CI failure) — Removed the ASF license comment block from both `docs/en/introduction/concepts/lineage.md` and `docs/zh/introduction/concepts/lineage.md`, so front matter goes straight into the `# ` heading like every other doc in the tree. Confirmed locally: `MarkdownTest.testPrimaryHeadersHaveNoTextAbove` now passes for both files. Also picked up two of the non-blocking suggestions while I was in there: **Issue 5** — Documented the worst-case blocking bound (`(retryTimes + 1) * timeoutMs`) on `LineageBackend#emit`'s Javadoc, so a future caller can judge whether to keep it off a latency-sensitive thread. **Issue 6** — `LineageConfig#asBoolean` now rejects anything that isn't `"true"`/`"false"` (case-insensitive) with a named `IllegalArgumentException`, matching the numeric-option error style, instead of silently folding a typo into `false`. Added a test for it. **Issue 3** (option-key/default duplication between `EnvCommonOptions` and `LineageConfig`) and **Issue 4** (connector-specific parsing in `LineageDatasetFactory`) are still open — left them out of this pass since you flagged both as non-blocking, and Issue 3's cross-check test in particular reaches into places this PR doesn't otherwise touch. Happy to add the Option A cross-check test for Issue 3 here if you'd rather see it land before merge instead of as a fast-follow. -- 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]
