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

[SPARK-35380][SQL] Loading SparkSessionExtensions from ServiceLoader #32515

Closed
wants to merge 20 commits into from
Closed

[SPARK-35380][SQL] Loading SparkSessionExtensions from ServiceLoader #32515

wants to merge 20 commits into from

Conversation

yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented May 12, 2021 •
edited
Loading

What changes were proposed in this pull request?

In yaooqinn/itachi#8, we had a discussion about the current extension injection for the spark session. We've agreed that the current way is not that convenient for both third-party developers and end-users.

It's much simple if third-party developers can provide a resource file that contains default extensions for Spark to load ahead

Why are the changes needed?

better use experience

Does this PR introduce any user-facing change?

no, dev change

How was this patch tested?

new tests

@github-actions github-actions bot added the SQL label May 12, 2021
@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42942/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Test build #138420 has finished for PR 32515 at commit baa61a3.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42949/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42949/

@yaooqinn yaooqinn changed the title [WIP][SPARK-35380][SQL] Loading SparkSessionExtensions from ServiceLoader [SPARK-35380][SQL] Loading SparkSessionExtensions from ServiceLoader May 12, 2021
@yaooqinn
Copy link
Member Author

cc @cloud-fan @MrPowers @HyukjinKwon @maropu thanks

@SparkQA
Copy link

SparkQA commented May 12, 2021

Test build #138434 has finished for PR 32515 at commit 4e84a6f.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yaooqinn
Revert "avoid API change"
453ad87 
This reverts commit 4e84a6f.
@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42955/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Test build #138443 has finished for PR 32515 at commit 453ad87.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42964/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42964/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Test build #138428 has finished for PR 32515 at commit ec731fc.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42971/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42971/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42973/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42973/

dongjoon-hyun
dongjoon-hyun approved these changes May 12, 2021
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you for updating, @yaooqinn .

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42982/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42982/

@SparkQA
Copy link

SparkQA commented May 12, 2021

Test build #138457 has finished for PR 32515 at commit e9b05cd.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 12, 2021

Test build #138461 has finished for PR 32515 at commit 9b5d7c2.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Ur, @yaooqinn . The R failure looks a little suspicious because it's consistent and relevant. It might be a side-effect in terms of test classes. Could you double-check it?

── 1. FAILURE (test_sparkSQL.R:4000:3): catalog APIs, listTables, listColumns, l
take(orderBy(f, "className"), 1)$className not equal to "org.apache.spark.sql.catalyst.expressions.Abs".
1/1 mismatches
x[1]: "noClass"
y[1]: "org.apache.spark.sql.catalyst.expressions.Abs"

@yaooqinn
Copy link
Member Author

Ur, @yaooqinn . The R failure looks a little suspicious because it's consistent and relevant. It might be a side-effect in terms of test classes. Could you double-check it?

── 1. FAILURE (test_sparkSQL.R:4000:3): catalog APIs, listTables, listColumns, l
take(orderBy(f, "className"), 1)$className not equal to "org.apache.spark.sql.catalyst.expressions.Abs".
1/1 mismatches
x[1]: "noClass"
y[1]: "org.apache.spark.sql.catalyst.expressions.Abs"

The failure proves this works well at the sparkR side, LOL. I changed the mocked function classname to fix this

@SparkQA
Copy link

SparkQA commented May 13, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43002/

@SparkQA
Copy link

SparkQA commented May 13, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43002/

yaooqinn and others added 2 commits May 13, 2021 11:18
@yaooqinn @HyukjinKwon
Update sql/core/src/main/scala/org/apache/spark/sql/SparkSessionExten…
e795f45 
…sionsProvider.scala

Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>
@yaooqinn @HyukjinKwon
Update sql/core/src/main/scala/org/apache/spark/sql/SparkSessionExten…
8f1be0e 
…sionsProvider.scala

Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>
@SparkQA
Copy link

SparkQA commented May 13, 2021

Test build #138484 has finished for PR 32515 at commit 8f1be0e.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 13, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43004/

@HyukjinKwon
Copy link
Member

Looks okay to me too

@SparkQA
Copy link

SparkQA commented May 13, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43009/

@SparkQA
Copy link

SparkQA commented May 13, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43009/

@yaooqinn yaooqinn self-assigned this May 13, 2021
@SparkQA
Copy link

SparkQA commented May 13, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43013/

@SparkQA
Copy link

SparkQA commented May 13, 2021

Test build #138482 has finished for PR 32515 at commit a79d76e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yaooqinn yaooqinn closed this in 5181543 May 13, 2021
@yaooqinn
Copy link
Member Author

Thanks for the help ~ @dongjoon-hyun @HyukjinKwon @cloud-fan

+1 for myself. merged to master

@SparkQA
Copy link

SparkQA commented May 13, 2021

Test build #138489 has finished for PR 32515 at commit ad18acc.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 13, 2021

Test build #138493 has finished for PR 32515 at commit b8b54ea.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yaooqinn yaooqinn deleted the SPARK-35380 branch March 10, 2022 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

@cloud-fan cloud-fan cloud-fan approved these changes

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees

@yaooqinn yaooqinn

Labels
EXAMPLES SQL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yaooqinn @SparkQA @dongjoon-hyun @HyukjinKwon @cloud-fan