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.

Sign up for GitHub

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

Jump to bottom

Added Shims for adding Databricks 14.3 Support [databricks] #11635

Open
wants to merge 20 commits into
base: branch-24.12
Choose a base branch
from
Open

Added Shims for adding Databricks 14.3 Support [databricks] #11635

wants to merge 20 commits into from
+6,995 −167

Conversation

razajafri
Copy link
Collaborator

This PR adds Shims for Databricks 14.3

@razajafri
Added 350db shims
8f7be42 
Signed-off-by: Raza Jafri <[email protected]>
@sameerz sameerz added the feature request New feature or request label Oct 19, 2024
@razajafri
Copy link
Collaborator Author

build

@mythrocks mythrocks mentioned this pull request Oct 21, 2024
Closed
@razajafri
Copy link
Collaborator Author

@gerashegalov @mythrocks PTAL

@gerashegalov gerashegalov self-requested a review October 30, 2024 00:26
gerashegalov
gerashegalov requested changes Oct 30, 2024
View reviewed changes
Copy link
Collaborator

@gerashegalov gerashegalov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please upmerge

I cannot build this PR branch. It seems to be missing the release350db profile etc.

datagen/src/main/spark320/scala/org/apache/spark/sql/tests/datagen/DataGenExprShims.scala Outdated Show resolved Hide resolved
...ugin/src/main/spark350db/scala/com/nvidia/spark/rapids/spark350db/RapidsShuffleManager.scala Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file should be dropped after upmerging to branch-24.12

...rc/main/scala/com/databricks/sql/transaction/tahoe/rapids/GpuOptimisticTransactionBase.scala Outdated Show resolved Hide resolved
@gerashegalov
Copy link
Collaborator

Please update Scala 2.13 poms

@razajafri
Copy link
Collaborator Author

@gerashegalov I have addressed your concerns. PTAL

gerashegalov
gerashegalov reviewed Nov 2, 2024
View reviewed changes
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM. at least one copyright to update

sql-plugin/src/main/spark320/scala/com/nvidia/spark/rapids/RapidsShuffleManager.scala Outdated Show resolved Hide resolved
gerashegalov
gerashegalov previously approved these changes Nov 4, 2024
View reviewed changes
@gerashegalov
Copy link
Collaborator

Re failing package-tests (350db143, false) , need to make sure that the db buildvers are not included in the Github Workflow build matrix

@razajafri
Copy link
Collaborator Author

Re failing package-tests (350db143, false) , need to make sure that the db buildvers are not included in the Github Workflow build matrix

Hang on, I will have to modify the workflow file and remove the change from pom.xml

@gerashegalov
Copy link
Collaborator

build

gerashegalov
gerashegalov previously approved these changes Nov 6, 2024
View reviewed changes
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

@razajafri
Copy link
Collaborator Author

build

@mythrocks
Copy link
Collaborator

It's looking good to go, now that the build is passing as well, no?

@NvTimLiu
Copy link
Collaborator

NvTimLiu commented Nov 7, 2024

@razajafri Can you enable Databricks-14.3 in the pre-merge CI pipeline in this PR as below?

https://github.com/NVIDIA/spark-rapids/blob/branch-24.12/jenkins/Jenkinsfile-blossom.premerge-databricks#L94

values '11.3', '12.2', '13.3', '14.3'

mythrocks
mythrocks previously approved these changes Nov 7, 2024
View reviewed changes
@razajafri
Copy link
Collaborator Author

build

@razajafri
Copy link
Collaborator Author

@razajafri Can you enable Databricks-14.3 in the pre-merge CI pipeline in this PR as below?

https://github.com/NVIDIA/spark-rapids/blob/branch-24.12/jenkins/Jenkinsfile-blossom.premerge-databricks#L94

values '11.3', '12.2', '13.3', '14.3'

I have updated it but this will turn the integration tests on. We don't want the integration tests to run against Databricks 14.3 because we aren't ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe left review comments

@mythrocks mythrocks mythrocks left review comments

@gerashegalov gerashegalov gerashegalov left review comments

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

@tgravescs tgravescs Awaiting requested review from tgravescs tgravescs is a code owner

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@razajafri razajafri

Labels
feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@razajafri @gerashegalov @mythrocks @NvTimLiu @jlowe @sameerz