DanielLeens commented on PR #11307: URL: https://github.com/apache/seatunnel/pull/11307#issuecomment-6266719729
Thanks @SEZ9 for the thorough follow-up on head `cadcf87e74`, and thanks @goutamadwant for all the iterations on this one. I re-checked the three items that are source-verifiable against this exact head, so the author has a precise to-do list: - **F8 (Spark 3.4.x guidance)** - this looks addressed at this head. `docs/en/engines/spark.md:19-21` now says 3.4 deployments should retain `seatunnel-spark-3-starter.jar` and the existing launcher, and that this change adds no separate 3.4 starter and extends no 3.4 coverage. I could not confirm the matching `docs/zh` and `quick-start-spark.md` wording in this pass, so a quick check that both are aligned would be appreciated. - **F4 (`log4j-slf4j-impl` plus `log4j-slf4j2-impl`)** - still open from what I can see. `seatunnel-core/seatunnel-spark-starter/pom.xml:79-80` still lists both in the logging include list, and the 3.5 starter excludes only `log4j-slf4j2-impl` from the Spark dependencies (`seatunnel-spark-3.5-starter/pom.xml:45,62,79`). Please either keep a single binding that matches the SLF4J version shipped, or add a short note explaining why both are safe on the launcher classpath. - **F7 (streaming on 3.5.8)** - I agree with SEZ9; I only find `Spark35RowEncoderTest` and `Spark35BatchWriteTest` for 3.5, and no streaming test. Please add one micro-batch case, or soften the docs so they do not point 3.5 users at the streaming template as verified. On CI, the required `Build` is still failing for `cadcf87e74`, and the branch is 30 commits behind `dev`. GitHub's job listing for the fork run returned an error when I tried to read it, so I cannot name the failed jobs for this head and will not guess. The previous head's failures were in jobs that do not touch the launcher code. Please sync with the latest `dev` and re-run only the failed jobs so we get a clean signal for the exact commit. The Windows packaged-launcher run and the quoting demo (config path with spaces and embedded quotes) that SEZ9 asked for are still the gating evidence before merge. I have no new source-level findings beyond the points above. Happy to do a final pass once those land. -- 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]
