SEZ9 commented on PR #11307: URL: https://github.com/apache/seatunnel/pull/11307#issuecomment-5230719623
Thanks @goutamadwant, that sounds like the right direction, and the `SparkRowEncoder` shim is a good intermediate step, but I agree with @liunaijie that the cleaner target shape is a separate Spark 3.5 track rather than a shimmed replacement of the 3.3 line. Concretely, what I'd like to see in the rework: 1. **Keep the existing Spark 3.3 starter/translation modules untouched**, and add Spark 3.5 as a parallel track, following the same pattern we already use to separate Spark 2 and Spark 3 (own version property, starter artifact, and translation path). That way existing 3.3/3.4 users see zero behavior change. 2. **E2E coverage for both lanes** — keep the current Spark 3 container lane green and add a new `apache/spark:3.5.8`-based lane, including the non-root image and `getDockerCommand()` null-safety points from my earlier review. 3. **Root pom property naming** — with a parallel track, please avoid renaming/removing the existing `spark.3.3.0.version` property so modules referencing it don't break; introduce a new property for the 3.5 line instead. 4. **Sync with latest `dev` before pushing** — the branch has repeatedly drifted behind upstream, and that's been muddying the CI signal on the broad matrix lanes. Once the reworked structure is up, I'll re-review from scratch. If CI is still red on the rebased head, please drop the fresh failing check link here and I'll help narrow it against that revision. Thanks again for sticking with this — the 3.5.8 support itself is clearly wanted (#11128), we just want to land it without dropping the existing baseline. <!-- streview-comment:95 --> -- 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]
