SEZ9 commented on PR #11307:
URL: https://github.com/apache/seatunnel/pull/11307#issuecomment-6245156746
Thanks for the follow-up on `d55343e8cb`. Looking at the completed `Build`
run `38091447278`, the failed jobs (`unit-test (11)`, `engine-k8s-it (11/17)`,
`elasticsearch-connector-it (11/17)`, `all-connectors-it-6 (17)`,
`doris-connector-it (17)`, `rocketmq-connector-it (11)`) don't touch the Spark
launcher code this PR changes, so they look environmental rather than caused by
this diff. Please sync with the latest `dev` and re-run the failed jobs so we
get a clean signal.
That said, green CI doesn't close out the earlier source-level points, since
none of them are exercised by those jobs. Before this can move forward, I still
need the following addressed or explicitly answered here:
1.
**`seatunnel-core/seatunnel-spark-starter/seatunnel-spark-3.5-starter/src/main/bin/start-seatunnel-spark-3.5-connector-v2.cmd`
(HIGH)** — `setlocal disabledelayedexpansion` is issued before
`!errorlevel!`/`!CMD!` are expanded, so the branches compare literal strings
and `call` targets the literal text `!CMD!`. Please fix the expansion ordering
and confirm you've run a job through this script on Windows.
2. **Same `.cmd` file** — the script exits 0 when the starter java process
fails with no stdout, and `for /f` never exposes java's real exit code. Please
propagate the real exit code.
3.
**`seatunnel-core/seatunnel-spark-starter/seatunnel-spark-3.5-starter/src/main/bin/start-seatunnel-spark-3.5-connector-v2.sh`**
— the launcher `eval`s a command line assembled from job-config values. Please
avoid `eval` here (e.g. build an array / pass arguments directly), and quote
`args=$@`, `${args}` and `${CLASS_PATH}` so argument quoting and globbing are
preserved.
4.
**`seatunnel-core/seatunnel-spark-starter/seatunnel-spark-3.5-starter/pom.xml`**
— the starter shades `seatunnel-translation-spark-3.3` (compiled against Spark
3.3.0). Please describe how Catalyst binary compatibility with 3.5.8 was
verified, since any mismatch only surfaces at runtime as `NoSuchMethodError`.
5. **`seatunnel-core/seatunnel-spark-starter/pom.xml`** — both
`log4j-slf4j-impl` and the new `log4j-slf4j2-impl` are in the logging include
list while both launchers put `starter/logging/*` on the classpath. Please keep
only the binding that matches the SLF4J version actually shipped.
6. **Tests** — the new Spark 3.5 tests cover the row encoder and batch write
only, while the docs point 3.5 users at the streaming template. Please add a
micro-batch streaming test against 3.5.8, or explain why it isn't needed.
7. **Docs** (`docs/en/engines/spark.md`,
`docs/en/getting-started/locally/quick-start-spark.md`) — the text narrows
`seatunnel-spark-3-starter.jar` to "Spark 3.3.x" and leaves Spark 3.4.x users
without guidance. Please clarify which starter 3.4.x users should pick.
If any of these were already handled in `d55343e8cb`, a short per-item note
pointing to the change is enough — I'll re-review promptly once the rebase and
rerun are in.
<!-- streview-comment:1839 -->
--
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]
