SEPURI-SAI-KRISHNA commented on PR #12899: URL: https://github.com/apache/seatunnel/pull/12899#issuecomment-6345012622
Thanks for the detailed review, @DanielLeens. Both items are addressed on the new head. **Issue 1, the red CI.** Your instruction was to sync once #12875 lands. #12875 never landed: hutiefang76 closed it unmerged on 2026-10-22 with the branch conflicting. The same fix reached `dev` through #12938 instead (`cc54bb433`), which carried two unrelated repairs in one commit: the `AbstractMultiCatalogTransform` serialVersionUID fix for #12929, and 25 lines in `JdbcOptionRuleCompletenessTest` adding a `NOT_SINK_OPTIONS` exclusion for `numeric_special_value_as_null`, which is exactly the #12874 break your review traced. So the cause of the four red `unit-test` legs is gone from `dev`, by a different PR than either of us expected. I did not want to take that on trust, so I ran the formerly failing test locally on JDK 17: `LLMBooleanOutputTest` passes 42 tests, including `readsLegacyFactoryWrapperWithValidationDisabled`. This head is synced to `dev` at `4dcfa28cc`, so CI is now running on a base that contains both repairs. **Issue 2, the fully qualified `TransformException`.** Addressed, and I went slightly wider than your recommendation. Your issue text pointed at lines 653, 733 and 858 and suggested an import "whenever the file is next touched", while the recommended-fix line scoped it to the touched methods. Line 653 sits in a pre-existing method, so the narrow reading would have left the file holding an import next to seven fully qualified usages, which is a mixed style in one file. Since the file is being touched now, I converted all thirteen references. Every edit is the same substitution with no assertion change; `spotless` joined two `assertThrows` calls that became short enough for a single line, which is the only reason the line count moves. `ZetaSQLEngineTest.java`, the other test file in this PR, already used the import throughout, so nothing changed there. **Validation on this head**, JDK 17: - `seatunnel-transforms-v2`: 1493 tests, 0 failures, 0 errors, 0 skipped - `SystemFunctionTest`: 26 tests, 0 failures, 0 errors - `./mvnw spotless:check`: clean - `./mvnw -DskipTests verify -pl seatunnel-transforms-v2 -am`: clean I also confirmed the sync did not disturb the documentation: three `dev` commits touched the same two doc files, but in unrelated sections, and the BIGINT entry still sits directly under the INT entry it cross-references. CI is in flight on the new head and I will report what it says rather than assume it. -- 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]
