SEZ9 commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-6159590952
Thanks @hesam-oxe for the detailed checklist against `8b383c0` and for sharing your local run. I haven't yet been able to verify the changes against the diff myself, so I can't mark the findings resolved based on the summary alone. Before I do a full pass, two things would help: - **F1 (destination-key collisions)** — is there a test covering the collision case specifically: two sinks with equal identifiers but different sink classes/configs that must *not* share a writer? If it's already in the suite you ran, please name it; if not, a small case in `MultiTableSinkWriterTest` would be welcome. - **F2 (snapshot/restore duplication)** — similarly, is there a restore test that feeds legacy per-alias state through and asserts the writer receives the merged state exactly once? For F3, F4/F6, F5, F7 and F8, the described approach sounds in line with what the earlier review asked for; I'll confirm each against the actual changes when I go through the diff. The `seatunnel-config-shade` build-ordering note sounds like a pre-existing issue unrelated to this PR, so no action needed here. <!-- streview-comment:1735 --> -- 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]
