Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Databricks build scripts [databricks] #5588

Merged
merged 3 commits into from
May 24, 2022

Conversation

NvTimLiu
Copy link
Collaborator

@NvTimLiu NvTimLiu commented May 23, 2022

To fix #5535

1, Change unused Databricks build parameter build_profiles to mvn_opt, will re-use mvn_opt as maven options, e.g. mvn_opt='-Dspark-rapids-jnv.version=22.06.0-SNAPSHOT'

2, There would be parameters mismatching issue for the script build.sh, if empty parameters are used, or if one of the parameters are missing. e.g. "build.sh param1 'blankspace' param3 would be treat as "build.sh param1 param2"

To avoid this mismatching, pass parameters through shell environment for the script file build.sh instead, e.g.,
SPARK_VER=3.1.2 MVN_OPT=' ' SOURCE_FILE=spark-rapids.tgz bash build.sh

3, remove unused build_profiles parameter from Jenkinsfile-blossom.premerge

Signed-off-by: Tim Liu timl@nvidia.com

1, Change unused Databricks build parameter `build_profiles` to `mvn_opt`, will re-use mvn_opt as maven options, e.g. mvn_opt='-Dspark-rapids-jnv.version=22.06.0-SNAPSHOT'

2, There would be parameters mismatching issue for the script build.sh, if empty parameters are used, or if one of the parameters are missing. e.g. "build.sh param1  'blankspace'  param3  would be treat as "build.sh param1   param2"

To avoid this mismatching, pass parameters through shell environment for the script file build.sh instead, e.g.,
SPARK_VER=3.1.2 MVN_OPT=' ' SOURCE_FILE=spark-rapids.tgz bash build.sh

3, remove unused `build_profiles` parameter from Jenkinsfile-blossom.premerge

Signed-off-by: Tim Liu <timl@nvidia.com>
@NvTimLiu NvTimLiu added the build Related to CI / CD or cleanly building label May 23, 2022
@NvTimLiu NvTimLiu requested review from tgravescs, pxLi and zhanga5 May 23, 2022 10:22
@NvTimLiu NvTimLiu self-assigned this May 23, 2022
@@ -38,7 +38,10 @@ def main():
print("rsync command: %s" % rsync_command)
subprocess.check_call(rsync_command, shell = True)

ssh_command = "bash -c 'ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null ubuntu@%s -p 2200 -i %s %s %s %s %s %s 2>&1 | tee buildout; if [ `echo ${PIPESTATUS[0]}` -ne 0 ]; then false; else true; fi'" % (master_addr, params.private_key_file, params.script_dest, params.tgz_dest, params.base_spark_pom_version, params.base_spark_version_to_install_databricks_jars, params.build_profiles)
ssh_command = "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null ubuntu@%s -p 2200 -i %s " \
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid this mismatching, pass parameters through shell environment for the script file build.sh instead, e.g.,
SPARK_VER=3.1.2 MVN_OPT=' ' SOURCE_FILE=spark-rapids.tgz bash build.sh

@NvTimLiu
Copy link
Collaborator Author

build

'3.0.1'
],
'9.1': [
'9.1.x-gpu-ml-scala2.12',
'3.1.2',
'init_cudf_udf.sh',
'databricks312,!snapshot-shims',
'3.1.2'
],
'10.4': [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the below databricks321,!snapshot-shims should be removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, my fault, this changed of the line 321 was wrongly revoked in my local branch. Fixed

@NvTimLiu
Copy link
Collaborator Author

build

@@ -89,7 +90,7 @@ def usage():
elif opt in ('-v', '--basesparkpomversion'):
base_spark_pom_version = arg
elif opt in ('-b', '--bulidprofiles'):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change --bulidprofiles to --mvnoptions?

@NvTimLiu
Copy link
Collaborator Author

build

@tgravescs tgravescs merged commit a12eccc into NVIDIA:branch-22.06 May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to CI / CD or cleanly building
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Need to pass MVN_OPT parameter for Databricks build scripts supporting more maven options
3 participants