SEZ9 commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5882450937
@hesam-oxe thanks for the detailed status on 1bfcea62. On F1, F2, F4, F5 and F6, the approach you describe (folding the sink class and `getPhysicalDestinationIdentifier()` into `DestinationKey` with an object-identity fallback; emitting shared writer state once via `groupByIdentity` under the canonical identifier with `getRestoredState()` scanning the full alias set; the docs updates and `proxyContexts` fix; and the `CONTINUE_OTHER_TABLES` isolation in e3e57c19518e) sounds right, and `testSamePhysicalIdentifierDoesNotShareAcrossConnectorClasses` is good coverage for F1. I'll verify against the diff before marking them resolved. Three of the earlier findings aren't in your summary, so I want to make sure they aren't lost: 1. **F3 (MEDIUM)** — the shared writer is created from one alias's sink instance / `CatalogTable` but receives rows from every aliased table. Is there now a guard (or at least a clear log/exception) when schema or config diverges across the aliased shards, or is this intentionally left to the connector? Either is fine, but please say which and, if it's the latter, note the contract in the `getPhysicalDestinationIdentifier()` Javadoc/docs so implementers know. 2. **F7 (MEDIUM)** — the `IOException` from `createWriter`/`restoreWriter` being wrapped in an unchecked `RuntimeException` inside `computeIfAbsent`. Has this been reworked so the declared checked failure contract is preserved (e.g. build the writer outside `computeIfAbsent`, or unwrap and rethrow)? If it's already in 1bfcea62, just point me to it. 3. **F8 (LOW)** — the missing parameter/return tags on the `getDestinationKey` Javadoc. Trivial, but a quick confirmation would be appreciated. Once F3 and F7 are either addressed or explicitly justified, I'm ready to approve. Thanks for sticking with this one. <!-- streview-comment:1388 --> -- 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]
