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

Add Spark 3.3.4 Shim #10082

Merged
merged 14 commits into from
Jan 2, 2024
Merged

Conversation

razajafri
Copy link
Collaborator

@razajafri razajafri commented Dec 20, 2023

This PR adds shims for Spark 3.3.4

Changes Made:

  • Ran shimplify mvn generate-sources -Dshimplify=true -Dshimplify.move=true -Dshimplify.overwrite=true -Dshimplify.add.shim=334 -Dshimplify.add.base=333
  • Fixed the failing test for Sequence length
  • Moved some shims around to 334 folder which is the earliest version for those shims

Tests:
All IT tests were run

fixed #9096

Signed-off-by: Raza Jafri <[email protected]>
@razajafri
Copy link
Collaborator Author

The following tests were failing but I fixed them by pulling in changes from #9962

This may cause a merge conflict depending on what gets checked in first

==================================================================================================================================================================================================== short test summary info ====================================================================================================================================================================================================
FAILED ../../src/main/python/collection_ops_test.py::test_sequence_too_long_sequence[Integer(not_null)][DATAGEN_SEED=1703023161, INJECT_OOM] - AssertionError: Expected error 'Too long sequence' did not appear in 'py4j.protocol.Py4JJavaError: An error occurred while calling o271142.collectToPython.
FAILED ../../src/main/python/collection_ops_test.py::test_sequence_too_long_sequence[Long(not_null)][DATAGEN_SEED=1703023161] - AssertionError: Expected error 'Too long sequence' did not appear in 'py4j.protocol.Py4JJavaError: An error occurred while calling o271226.collectToPython.
============================================================================================================================================================== 2 failed, 19947 passed, 1732 skipped, 371 xfailed, 303 xpassed, 8532 warnings in 4448.90s (1:14:08) ==============================================================================================================================================================

@razajafri
Copy link
Collaborator Author

build

1 similar comment
@razajafri
Copy link
Collaborator Author

build

@razajafri razajafri marked this pull request as draft December 20, 2023 17:25
@sameerz sameerz added the feature request New feature or request label Dec 23, 2023
@razajafri razajafri marked this pull request as ready for review December 29, 2023 22:26
@razajafri
Copy link
Collaborator Author

build

Copy link
Contributor

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Whitespace nits, lgtm.

@@ -14,6 +14,7 @@
* limitations under the License.
*/


Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace-only change to this file?

@@ -14,6 +14,7 @@
* limitations under the License.
*/


Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace-only change to this file?

@@ -14,6 +14,7 @@
* limitations under the License.
*/


Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace-only change to this file?

@razajafri
Copy link
Collaborator Author

Thanks for the review @jlowe I will push a separate PR for the whitespace nits as the CI has passed.

@razajafri razajafri merged commit d39c63a into NVIDIA:branch-24.02 Jan 2, 2024
38 of 39 checks passed
@razajafri razajafri deleted the SP-9096-334-shim branch January 2, 2024 19:18
@jlowe
Copy link
Contributor

jlowe commented Jan 2, 2024

I will push a separate PR for the whitespace nits as the CI has passed.

The point of the comment was to reduce unnecessary file churn by needlessly touching a file. Pushing out the fix for that to a separate PR misses the point and increases the unnecessary file churn. I was fine with it going unfixed as it's pretty minor, not sure it needs to be cleaned up now that it's been done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Add Spark 3.3.4 support
3 participants