DanielLeens commented on PR #11060:
URL: https://github.com/apache/seatunnel/pull/11060#issuecomment-5409116093
Thanks for the deep dive, @zhangshenghang — I re-verified all three findings
against the current head (`5eb4e0002`) directly in the source and they all hold
up; replies with the specific file/line evidence are inline on each thread.
Updating my own record for anyone following this thread: my last full review
(2026-08-18) approved this PR on the wrapper's runtime/checkpoint/restore path,
but I did not re-derive the catalog-factory-id resolution
(`plugin_name.replace("-CDC", "")` → `OceanBase` → `OceanBaseCatalogFactory`,
which requires `compatible_mode`) myself, and none of my prior rounds caught
that this required key is undeclared in `optionRule()` and absent from both doc
task examples. That's a real gap across several of my own approvals, not just
this latest one — sorry for the miss, and thanks for tracking it down with an
actual reproduction.
Net: the [P1] `compatible_mode` contract break is a genuine blocker (breaks
the documented task example, breaks static validation of the E2E's own conf,
and leaves the oracle-mode catalog/runtime mismatch unconstrained). [P2]
restoreSource duplication and [P3] the E2E scope/driver notes are legitimate
but non-blocking, per my inline replies.
No new commit has landed since your review, so I'm not submitting a fresh
formal review on this pass — once the `compatible_mode` fix (and ideally the P2
extraction) lands, this should get a clean full re-review from me.
--
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]
