SEZ9 commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5691955276
Thanks for reposting the head-by-head mapping for `ef2bb0955f30bc108ef96d30e9c4adfe1f294279` and for pointing me to review `5196154801` — I had not folded that one in. Going item by item against the previous F1–F8 scope, with the caveat that I'll confirm each against the diff rather than closing on description alone: - **F1 (destination-key collisions)** – Folding `sink.getClass()` plus the connector-supplied identifier into `DestinationKey.equals()/hashCode()`, with an object-identity fallback, is the right shape. I'll verify `testSamePhysicalIdentifierDoesNotShareAcrossConnectorClasses` in the diff before closing. - **F2 (snapshot fan-out / restore-time union)** – Persisting under one canonical identifier and merging legacy per-alias checkpoints by content in `getRestoredState()` is the intended fix. I'll check `testSharedWriterRoundTripRestoresOneCanonicalState` and `testRestoreMergesStateFromAllAliasedTables` in the diff before marking it closed. - **F3 (schema/config divergence)** – `validateSharedDestinationSchemas()` failing fast at construction sounds right; I'll confirm the check in `a9d8464` and keep the release-note callout in the PR description. - **F4 / F6 (docs)** – If `docs/en/developer/sink-connector-development.md` and the `zh` counterpart cover `getPhysicalDestinationIdentifier()` and the merged-state `restoreWriter` contract, that satisfies both. Could you confirm both files are in this PR rather than a follow-up? - **F5 (`proxyContexts` / `containsValue`)** and **F7 (`IOException` wrapped inside `computeIfAbsent`)** – Reported as not reproducible on this head. Could you say whether these were removed in `a9d8464` or were never present on the branch I reviewed? A one-line pointer is enough. - **F8 (Javadoc tags)** – Pending a quick look at the `getDestinationKey` Javadoc in the diff. Your note that `ef2bb0955f3` is a pure `dev`-sync merge appears to have been cut off at the end of the comment — could you re-post that part? Once the test names, both doc files, and the F5/F7 provenance are confirmed, I'll do a final pass limited to these eight items. <!-- streview-comment:1095 --> -- 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]
