DanielLeens commented on PR #12079: URL: https://github.com/apache/seatunnel/pull/12079#issuecomment-5805501481
SEZ9, the "detailed walkthrough of `d8045271fcf6` against `6dcbe452af3`" you're referring to is my own last review (submitted 2026-09-22T10:29:46Z, `APPROVED`), which is still current - `d8045271fcf6` remains the PR's head, no new commit has landed since. That review already answers each of your six points with exact file:line citations, so here they are pulled out directly rather than making you re-derive them from the review body: - **F1 (duplicate output IDs):** `TransformDependencyScheduler.java:116`, `waitingByInputId.remove(transform.outputId)` - `remove` (not `get`) empties the entry the first time any producer of that ID is scheduled, so a second producer of the same output releases nothing further. Tests: `ConfigParserUtilTest#testDuplicateOutputsReleaseEachMissingInputOnlyOnce` (`ConfigParserUtilTest.java:356`) and `MultipleTableJobConfigParserTest#testDuplicateUnnamedOutputsWaitForOtherJoinInput`, which asserts the joined transform's actual upstream actions. - **F2/F4 (omitted `plugin_input`):** `ScheduledTransform.inputOmitted` (`TransformDependencyScheduler.java:236`) is `!readonlyConfig.getOptional(PLUGIN_INPUT).isPresent()`, folded into `emptyInputFallback = transform.inputIds.isEmpty() || transform.inputOmitted` at line 140 - this applies regardless of transform count, not just single-transform jobs. Tests: `MultipleTableJobConfigParserTest#testTerminalOmittedInputUsesNamedTransformSchemaAndEdge` and `#testOmittedInputPrefersExistingDefaultOverLastNamedOutput`, plus the dry-run mirror `SeaTunnelConfValidateCommandTest#testConnectDryRunOmittedInputPrefersExistingDefault`. - **F3/F5 (shared scheduler):** Confirmed structurally, not just by claim - `MultipleTableJobConfigParser` has zero remaining scheduler code of its own, and `DryRunConnectValidator.scheduleTransforms` (`DryRunConnectValidator.java:228-234`) is a 6-line wrapper calling the same `TransformDependencyScheduler.scheduleTransforms`. `getInputIds` is likewise single-sourced at `ConfigParserUtil.java:269`. - **F6 (single-transform self-cycle):** Yes - `ConfigParserUtil.checkGraph`'s simple-graph branch (`:72-78`) now routes even 0/1-transform graphs through `scheduleTransforms` before `checkSimpleGraph`. An explicit self-reference has `inputOmitted=false`, so it is rejected with `Transform dependency cycle detected: x -> x`. Tests: `ConfigParserUtilTest#testSimpleGraphRejectsExplicitSelfCycle`, `#testSingleExplicitSelfCycleCannotUseLegacyFallback`, `SeaTunnelConfValidateCommandTest#testConnectDryRunRejectsSingleExplicitSelfCycle`, and the client-to-master `JobExecutionIT#testRejectsExplicitTransformSelfCycleBeforeSubmission` (bounded by `assertTimeoutPreemptively`, `JobExecutionIT.java:101`). - **F7 (docs):** Both `docs/en` and `docs/zh` `incompatible-changes.md` describe the fail-fast rejection of cyclic/unresolved transform graphs, re-verified against the current code. As a bonus closure: `--dry-run connect` now also surfaces the specific `Transform dependency cycle detected: a -> b -> a` message (not just the old generic one), since `checkTransformCycles` runs inside the shared `scheduleTransforms` (`TransformDependencyScheduler.java:135`) that `DryRunConnectValidator` calls too. - **F8 (helper duplication / `getOutputId()`):** The redundant static helpers were removed in the consolidation. `getOutputId()` is confirmed to still have no production caller (test-only), which we already agreed to track as a non-blocking follow-up rather than a blocker. Nothing new needed here - happy to expand on any single item if you want more than the pointer. -- 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]
