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

Install cuDF-py against python 3.10 on Databricks #11477

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

NvTimLiu
Copy link
Collaborator

Fix on Databricks runtime for : #11394

Enable the udf_cudf_test test case for Databricks-13.3

Rapids 24.10+ drops python 3.9 or below conda packages. ref: https://docs.rapids.ai/notices/rsn0040/

Install cuDF-py packages against python 3.10 and above on Databricks runtime to run UDF cuDF tests, because on DB-13.3 Conda is not installed by default.

Fix on Databricks runtime for : NVIDIA#11394

Enable the udf_cudf_test test case for Databricks-13.3

Rapids 24.10+ drops python 3.9 or below conda packages. ref: https://docs.rapids.ai/notices/rsn0040/

Install cuDF-py packages against python 3.10 and above on Databricks runtime to run UDF cuDF tests, because on DB-13.3 Conda is not installed by default.

Signed-off-by: timl <[email protected]>
@NvTimLiu NvTimLiu added the build Related to CI / CD or cleanly building label Sep 18, 2024
@NvTimLiu NvTimLiu self-assigned this Sep 18, 2024
@@ -61,9 +47,41 @@ REQUIRED_PACKAGES=(
requests
sre_yield
)
if base=$(conda info --base); then
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Support install cudf-py packages via either Conda or PIP.

Copy link
Collaborator

@pxLi pxLi Sep 18, 2024

Choose a reason for hiding this comment

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

Q: if no conda in PATH this would error out? use detect existence + var assignment in the same expression is not recommended here

can we try to use the the command to check if exist first? this would help make the if expression and else branches more clear

if command -v conda >/dev/null 2>&1; then

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does any of Databricks Runtime 13.3+ have conda? If no, I think we can drop supporting conda installation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Q: if no conda in PATH this would error out?

The if expression will not error out but get 'false' value instead.

can we try to use the the command to check if exist first? this would help make the if expression and else branches more clear

if command -v conda >/dev/null 2>&1; then

Sounds good to me, let me update it to make the if/else more readable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does any of Databricks Runtime 13.3+ have conda?

DB-13.3+ do not install conda by default

If no, I think we can drop supporting conda installation.

I'd like to keep conda installation there for some time, because:
Keep it to maintain compatibility with DB13.3 or earlier conda installations.
We also have the jenkins/databricks/cudf_udf_test.sh script, which is still compatible with conda installations.

Copy link
Collaborator

@pxLi pxLi left a comment

Choose a reason for hiding this comment

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

LGTM

@NvTimLiu
Copy link
Collaborator Author

build

@NvTimLiu
Copy link
Collaborator Author

The pre-merge CI can not cover the udf-cudf python tests,

Verified the PR locally, both the init script and udf-cudf tests got PASS:

image

@pxLi
Copy link
Collaborator

pxLi commented Sep 19, 2024

failed unrelated cases: test_broadcast_join_with_conditionals in scala213

could be related to latest cudf change

@NvTimLiu
Copy link
Collaborator Author

build

@pxLi pxLi merged commit 510ee83 into NVIDIA:branch-24.10 Sep 24, 2024
44 of 45 checks passed
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.

3 participants