[SPARK-23809][SQL] Active SparkSession should be set by getOrCreate #20927
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericl
commented
Mar 28, 2018
| @@ -34,6 +34,9 @@ private[spark] class TestSparkSession(sc: SparkContext) extends SparkSession(sc) | |||
| this(new SparkConf) | |||
| } | |||
|
|
|||
| SparkSession.setDefaultSession(this) | |||
There was a problem hiding this comment.
Intentional conflict with #20926 , should resolve once one is merged
There was a problem hiding this comment.
We will merge that one first.
|
LGTM |
|
Test build #88681 has finished for PR 20927 at commit
|
felixcheung
approved these changes
Mar 29, 2018
|
Per @marmbrus 's suggestion also added a |
|
Test build #88723 has finished for PR 20927 at commit
|
|
Test build #88724 has finished for PR 20927 at commit
|
|
@ericl Address the conflict? |
|
@gatorsmile done! |
|
LGTM pending Jenkins |
|
Test build #88856 has finished for PR 20927 at commit
|
|
Thanks! Merged to master |
gatorsmile
reviewed
Apr 4, 2018
| * Returns the currently active SparkSession, otherwise the default one. If there is no default | ||
| * SparkSession, throws an exception. | ||
| * | ||
| * @since 2.4.0 |
There was a problem hiding this comment.
@ericl Could you submit a PR to Spark 2.3 without this new API?
ericl
added a commit
to ericl/spark
that referenced
this pull request
Apr 4, 2018
## What changes were proposed in this pull request? Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists. The semantics here can be cleaned up if we also set the active session when the default session is set. Related: https://github.com/apache/spark/pull/20926/files ## How was this patch tested? Unit test, existing test. Note that if apache#20926 merges first we should also update the tests there. Author: Eric Liang <[email protected]> Closes apache#20927 from ericl/active-session-cleanup.
robert3005
pushed a commit
to palantir/spark
that referenced
this pull request
Apr 4, 2018
## What changes were proposed in this pull request? Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists. The semantics here can be cleaned up if we also set the active session when the default session is set. Related: https://github.com/apache/spark/pull/20926/files ## How was this patch tested? Unit test, existing test. Note that if apache#20926 merges first we should also update the tests there. Author: Eric Liang <[email protected]> Closes apache#20927 from ericl/active-session-cleanup.
mshtelma
pushed a commit
to mshtelma/spark
that referenced
this pull request
Apr 5, 2018
## What changes were proposed in this pull request? Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists. The semantics here can be cleaned up if we also set the active session when the default session is set. Related: https://github.com/apache/spark/pull/20926/files ## How was this patch tested? Unit test, existing test. Note that if apache#20926 merges first we should also update the tests there. Author: Eric Liang <[email protected]> Closes apache#20927 from ericl/active-session-cleanup.
asfgit
pushed a commit
that referenced
this pull request
Apr 8, 2018
…OrCreate This backports #20927 to branch-2.3 ## What changes were proposed in this pull request? Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists. The semantics here can be cleaned up if we also set the active session when the default session is set. Related: https://github.com/apache/spark/pull/20926/files ## How was this patch tested? Unit test, existing test. Note that if #20926 merges first we should also update the tests there. Author: Eric Liang <[email protected]> Closes #20971 from ericl/backport-23809.
otterc
pushed a commit
to linkedin/spark
that referenced
this pull request
Mar 22, 2023
Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists. The semantics here can be cleaned up if we also set the active session when the default session is set. Related: https://github.com/apache/spark/pull/20926/files Unit test, existing test. Note that if apache#20926 merges first we should also update the tests there. Author: Eric Liang <[email protected]> Closes apache#20927 from ericl/active-session-cleanup. (cherry picked from commit 359375e) NOTE: This cherry-pick includes only some of the original changes, because LIHADOOP-54684 already made some identical changes as part of test resolution. This now ensures the entirety of the original SPARK-23809 code is backported. Also note that when SPARK-23809 was originally backported to branch-2.3 in PR apache#20971, the new `active` API was _not_ included since new APIs shouldn't generally be added in patch releases. That new API is exactly what is needed, so we backport directly from the original commit. RB=2575134 BUG=BDP-6088 G=spark-reviewers R=mmuralid,wyzhang,smahadik A=mmuralid,wyzhang
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists.
The semantics here can be cleaned up if we also set the active session when the default session is set.
Related: https://github.com/apache/spark/pull/20926/files
How was this patch tested?
Unit test, existing test. Note that if #20926 merges first we should also update the tests there.