SEZ9 commented on PR #12079: URL: https://github.com/apache/seatunnel/pull/12079#issuecomment-6407882841
Thanks for the follow-up on the F1–F8 points. Based on the latest automated pass at `9152a91fa278e7827e66b73c691e6f1e8f961abc`, the direction looks right: a single indexed dependency scheduler shared by graph validation, runtime parsing and the connect dry-run (which is exactly what F5 asked for), waiting on every distinct dependency before plugin discovery, explicit cycle/unresolved rejection, and EN/ZH migration notes for the behavior change (F7). A few things I still want to confirm before approving, since the review summary I have is truncated and I'd rather see it in the PR than assume: 1. **F1 (duplicate output IDs):** Please point me to the unit test that covers two or more transforms omitting `plugin_output` (both defaulting to the same ID) feeding a multi-input downstream, and confirm the scheduler now counts *distinct* dependency IDs so a dependent is released exactly once. 2. **F2 / F4 (legacy fallback compatibility):** Please link the test(s) showing a multi-transform job whose last transform omits `plugin_input` still resolves via the terminal fallback, and clarify which previously-passing multi-transform shapes (if any) now intentionally fail with `ConfigCheckException`. Whatever is intentionally rejected should be spelled out in the migration notes. 3. **F3 / F8:** Confirm that scheduling and validation in `DryRunConnectValidator` now derive input IDs from the same resolver (no separate `getTransformInputIds` vs `getInputIds` paths), and that the unused `getOutputId()` accessor and the redundant `ReadonlyConfig` re-parsing were removed or are now actually used. 4. **F6 (single-transform self-cycle):** Please confirm there is a test asserting that a single transform whose `plugin_input` equals its own output ID is rejected by the dry-run with the same error as the runtime parser, rather than passing through the legacy fallback. 5. **F7 (docs):** The automated pass also flags a remaining documentation gap for the newly public resolver/wrapper API. Please add that to the same docs change so the user-facing and developer-facing notes land together. Once those are visible in the PR (tests + a short pointer to the doc sections), I'm happy to do the final pass. <!-- streview-comment:2048 --> -- 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]
