-
Notifications
You must be signed in to change notification settings - Fork 169
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
build: Move shim directories #318
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @kazuyukitanimura
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the original idea in the ticket is to create different shim for different Spark version instead of using reflection API for all Spark versions?
Thanks @andygrove @viirya
That's right. Currently it is |
I think the original idea is to have spark-3.2 and spark-3.3 shims which use reflection API, and spark-3.4 which uses Spark API as it is available. So for the Spark versions which have the API available, its shim layer doesn't need to use reflection. That's what I got from the ticket. I'm not sure if you are going to do that? If so, then it is fine to have this as initial move. |
My focus would be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as an initial move
Merged. Thanks. |
Thank you all! |
Which issue does this PR close?
Closes #140
Rationale for this change
To add Spark 3.5 and 4.0 shims later.
What changes are included in this PR?
This PR moves the shims to spark version specific directories
How are these changes tested?
Existing tests