Abacn commented on PR #38212:
URL: https://github.com/apache/beam/pull/38212#issuecomment-4269295806

   Thanks, sorry for late response. Since this is a large change here are some 
generic comments
   
   * I would like to take this opportunity to make Spark runner source file can 
be shared among versions, like what we've been doing for Flink runner: 
   
   
https://github.com/apache/beam/blob/621354f992be52eef68b5ce368a851f0f7511537/runners/flink/flink_runner.gradle#L73
   
   and other source override logics in the build.gradle
   
   Basically, the lowest version (3) is built on top of runners/spark/src/, the 
higher version (4) combines `runners/spark/src/` and `runners/spark/src/4` into 
a "build/source-override" with the latter overrides the former. By doing this 
can make the diff much smaller and easier to maintain (generic fixes only needs 
to change one version)
   
   We can copy-paste the logic from runners/flink/flink_runner.gradle#L73
   
   It's fine to have single or a few commits. 5+ commits won't further help. 
Alternatively we can split the support into multiple PRs, first check in 
spark-runner.gradle change and classic Spark runner, set 
applyJavaNature(publish=False) while work in progress. Then complete the 
support, and then job-server support


-- 
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]

Reply via email to