Disable spark snapshot shims pre-merge build in 22.04 #5079
Conversation
Signed-off-by: Peixin Li <pxli@nyu.edu>
pxLi
requested review from
jlowe,
revans2,
tgravescs,
GaryShen2008 and
NvTimLiu
as code owners
|
build |
jenkins/spark-premerge-build.sh
Outdated
| @@ -43,17 +43,20 @@ mvn_verify() { | |||
| env -u SPARK_HOME mvn -U -B $MVN_URM_MIRROR -Dbuildver=311cdh clean install -Drat.skip=true -DskipTests -Dmaven.javadoc.skip=true -Dskip -Dmaven.scalastyle.skip=true -Dcuda.version=$CUDA_CLASSIFIER -pl aggregator -am | |||
| env -u SPARK_HOME mvn -U -B $MVN_URM_MIRROR -Dbuildver=312 clean install -Drat.skip=true -DskipTests -Dmaven.javadoc.skip=true -Dskip -Dmaven.scalastyle.skip=true -Dcuda.version=$CUDA_CLASSIFIER -pl aggregator -am | |||
| env -u SPARK_HOME mvn -U -B $MVN_URM_MIRROR -Dbuildver=313 clean install -Drat.skip=true -DskipTests -Dmaven.javadoc.skip=true -Dskip -Dmaven.scalastyle.skip=true -Dcuda.version=$CUDA_CLASSIFIER -pl aggregator -am | |||
| env -u SPARK_HOME mvn -U -B $MVN_URM_MIRROR -Dbuildver=314 clean install -Drat.skip=true -DskipTests -Dmaven.javadoc.skip=true -Dskip -Dmaven.scalastyle.skip=true -Dcuda.version=$CUDA_CLASSIFIER -pl aggregator -am | |||
| # skip spark snapshot shims build in 22.04. (TODO) revert this commit in 22.06 after auto-merged | |||
There was a problem hiding this comment.
Let us formalize when the snapshots build are disabled so we don't have to do it over and over manually by commenting and uncommenting. This could be an environment variable or a dedicated configuration file in the repo.
Maybe we can reuse .github/workflows/auto-merge.yml ? Whether the snapshot builds are needed is directly related to auto-merge as this comment indicates. Could we make the current branch name's equality to the BASE in .github/workflows/auto-merge.yml
BASE_BRANCH=$(grep 'BASE: ' .github/workflows/auto-merge.yml | grep -o branch-.*)
CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
if [[ "$CURRENT_BRANCH" == "$BASE_BRANCH" ]]; then
# build snapshots
...
fiThere was a problem hiding this comment.
If so, we could simply based on https://github.com/NVIDIA/spark-rapids/blob/branch-22.06/jenkins/version-def.sh#L31 instead of a workflow file. let me refactor this
BTW the commit of pre-merge CI is the merged commit (merge feature to dev branch )
pxLi
force-pushed
the
ignore-pre-merge-snapshot-build-22.04
branch
from
1678ea1 to
0795738
Compare
|
build |
| @@ -121,6 +121,13 @@ nvidia-smi | |||
|
|
|||
| . jenkins/version-def.sh | |||
|
|
|||
| BUILD_SNAPSHOTS="false" | |||
| PREMERGE_PROFILES="-PnoSnapshots,pre-merge" | |||
| if [[ ${PROJECT_VER} =~ ^22\.06\. ]]; then # enable snapshot builds for active development branch only | |||
There was a problem hiding this comment.
The proposal to reuse auto-merge has the advantage is that it makes auto-merge config the single source of truth. And as part of our standard practice we never forget to update it .
With this solution there will be two places to update when the current branch enters the burn-down state. On the other hand it gives us the flexibility to manage auto-merge and BUILD_SHIMS independently but in a simplified form. I am not sure we have a need for this flexibility.
There was a problem hiding this comment.
And as part of our standard practice we never forget to update it .
Actually version-def.sh is the source of truth for testing (we source it in all our CI scripts). auto-merge BASE could probably be following other naming patterns that do not reflect versions
|
I agree it would be nice to have one mechanism, although it would also be nice to combine build scripts as well. We have build all and then we have separate things for premerge and nightly. That might be bigger changes though. can we reuse that. If we can minimize where we have to change in adding/removing snapshots that would be ideal. Perhaps we need a short term solution for this and then longer term bigger change. |
It has been used in all jenkins build and test scripts to simply export helpful ENVs
That file is for tests that do not need to clone the plugin github repo (just need some ENVs), usually used in pipelines which just required to download artifacts and run tests.
@tgravescs yes, created a ticket to track #5086. For now, I think we can leverage version-de.sh here. Please let me know if you have any other concern, thanks! |
What exactly do you mean by this? Adding snapshot versions to version-def.sh? is there other changes to this PR to do that? |
This PR is intentional to disable snapshot builds in pre-release version. So currently we take,
The plan is to set a single source of truth like the dist/pom.xml file to get above info #5086 |
Signed-off-by: Peixin Li pxli@nyu.edu
disable some shims build for spark snapshot versions in pre-merge.
nightly build and tests have be taken care of internally.