DanielLeens commented on PR #12079: URL: https://github.com/apache/seatunnel/pull/12079#issuecomment-6411621636
Hi @SEZ9 and @goutamadwant, thanks for the detailed questions. I re-read head `9152a91fa278` (static read only, nothing run locally). Here are the test pointers you asked for. 1. **F1 (duplicate output IDs):** `ConfigParserUtilTest#testDuplicateOutputsReleaseEachMissingInputOnlyOnce` (:356) and `#testDuplicateOutputsCannotHideUnproducedInput` (:260) cover the scheduler. `MultipleTableJobConfigParserTest#testDuplicateUnnamedOutputsWaitForOtherJoinInput` (:773) and `SeaTunnelConfValidateCommandTest#testConnectDryRunDuplicateUnnamedOutputsWaitForOtherInput` (:291) cover the runtime parser and the dry-run for two transforms omitting `plugin_output` that feed a multi-input downstream. Each output ID removes its waiting list once, so a dependent is released exactly once. 2. **F2 / F4 (fallbacks):** The terminal fallback is kept and tested in `ConfigParserUtilTest#testTerminalOmittedInputRetainsLegacyFallback` (:373), `MultipleTableJobConfigParserTest#testTerminalOmittedInputUsesNamedTransformSchemaAndEdge` (:728) and `#testTransformEvaluationOrderKeepsLegacySinkFallbackOnTerminalTransform` (:196). The shapes that now fail on purpose are covered by `ConfigParserUtilTest#testDoesNotApplyLegacyFallbackAfterAnotherTransformWasScheduled` (:175) and `#testRejectsAmbiguousEmptyInputFallbacks` (:194). Those shapes, and the savepoint-resubmission effect, are spelled out in the en and zh incompatible-changes entries. 3. **F3 / F8:** Input IDs come from the shared `ConfigParserUtil.getInputIds` in the scheduler, the runtime parser (`MultipleTableJobConfigParser.java:492`) and dry-run, so there is no second resolver. `ScheduledTransform.getOutputId()` is not unused: `DryRunConnectValidator.validateTransform` reads it. The runtime parser still re-reads the same IDs from config, which I rate as optional cleanup and not a correctness issue. 4. **F6 (self-cycle):** `ConfigParserUtilTest#testSingleExplicitSelfCycleCannotUseLegacyFallback` (:387), `SeaTunnelConfValidateCommandTest#testConnectDryRunRejectsSingleExplicitSelfCycle` (:260) and `JobExecutionIT#testRejectsExplicitTransformSelfCycleBeforeSubmission` cover it. Dry-run rethrows the scheduler failure as `ConfigCheckException` with the same message, so the wording matches the runtime `JobDefineCheckException`. 5. **F7 (docs):** The incompatible-changes entries are in place. You are right about the remaining gap, and it is the one item I still have open on this head: `ConfigParserUtil.getInputIds` (`ConfigParserUtil.java:269`, now `public`) and the new `scheduleTransforms` wrapper in `DryRunConnectValidator.java:228` have no contract comments. A short Javadoc on each (omitted `plugin_input` defaults to the default ID versus an explicit empty list, and the wrapper only translates the exception type) would close it. No algorithm change is needed. On CI: the Build for this head is a failure, but the one failing job I could inspect is `Run / connector-file-local-it (17, ubuntu-latest)`. It stopped within about 1.5 minutes with `Could not find or load main class org.apache.maven.wrapper.MavenWrapperMain`, before any test ran, so it looks like a runner or Maven-wrapper bootstrap problem and not this diff. The other non-green job, `connector-redis-it (11)`, was cancelled. Please rerun just those two failed jobs rather than the whole workflow. The branch also already contains the merged #12938 fix. So the only item between this PR and a clean source review is the two method comments, plus a green rerun. Thanks again for the careful work! -- 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]
