Skip to content

Conversation

@techaddict
Copy link
Contributor

What changes were proposed in this pull request?

Removing the withHiveSupport method of SparkSession, instead use enableHiveSupport

How was this patch tested?

ran tests locally

Copy link
Contributor

Choose a reason for hiding this comment

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

i think this one needs to be built?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I got you ? do you mean something like withSparkContext(sc) instead of getOrCreate()

Copy link
Contributor

Choose a reason for hiding this comment

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

In order to use this file, I think it needs to be built into a jar?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah we need to rebuild the jar. There's already a TODO on L31. Right now the corresponding test in HiveSparkSubmitSuite is ignored.

Copy link
Contributor Author

@techaddict techaddict May 3, 2016

Choose a reason for hiding this comment

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

Since we not longer create a assembly jar, not sure how to create this jar easily(steps could be found here #11630).

Copy link
Contributor

Choose a reason for hiding this comment

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

let's just fix it later

@andrewor14
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented May 3, 2016

Test build #57569 has finished for PR 12851 at commit 7a07671.

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

@SparkQA
Copy link

SparkQA commented May 3, 2016

Test build #2961 has finished for PR 12851 at commit 7a07671.

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

@rxin
Copy link
Contributor

rxin commented May 3, 2016

Seems like some legitimate failure/?

@techaddict
Copy link
Contributor Author

Jenkins retest this please.

@SparkQA
Copy link

SparkQA commented May 3, 2016

Test build #57656 has finished for PR 12851 at commit 7a07671.

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

@techaddict
Copy link
Contributor Author

techaddict commented May 4, 2016

@rxin yupp failed tests are sql hive tests, waiting for #12890

val builder = SparkSession.builder.config(conf)
if (SparkSession.hiveClassesArePresent) {
sparkSession = builder.enableHiveSupport().getOrCreate()
sparkSession = SparkSession.builder.enableHiveSupport().getOrCreate()
Copy link
Contributor

Choose a reason for hiding this comment

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

wait, this is wrong now. We want to use the builder we already have

@andrewor14
Copy link
Contributor

andrewor14 commented May 5, 2016

@techaddict a general suggestion is to try to get the SparkContext from the SparkSession rather than the other way round. Right now we call getOrCreate() and assume there's a SparkContext nearby, which may be true but is kind of brittle and outright wrong in some cases.

@SparkQA
Copy link

SparkQA commented May 5, 2016

Test build #57812 has finished for PR 12851 at commit 17e0703.

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

@SparkQA
Copy link

SparkQA commented May 5, 2016

Test build #57849 has finished for PR 12851 at commit 7ad494d.

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

@techaddict
Copy link
Contributor Author

@rxin @andrewor14 Comments Addressed, now all tests are passing 🎉

.setMaster("local")
.setAppName("testing")

val sparkSession = SparkSession.builder.enableHiveSupport().getOrCreate()
Copy link
Contributor

@andrewor14 andrewor14 May 5, 2016

Choose a reason for hiding this comment

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

this should use the builder's conf method. By the way for this file I would revert the changes here and fix it in #12924 instead, since @dilipbiswal is actually rebuilding the jar there.

@andrewor14
Copy link
Contributor

@techaddict this is getting really close. For the regression test for SPARK-8489 I would just fix it later since this patch doesn't rebuild that jar anyway.

@andrewor14
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented May 5, 2016

Test build #57904 has finished for PR 12851 at commit 5e464ad.

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

@andrewor14
Copy link
Contributor

Merging into master 2.0

@asfgit asfgit closed this in ed6f3f8 May 5, 2016
asfgit pushed a commit that referenced this pull request May 5, 2016
## What changes were proposed in this pull request?
Removing the `withHiveSupport` method of `SparkSession`, instead use `enableHiveSupport`

## How was this patch tested?
ran tests locally

Author: Sandeep Singh <[email protected]>

Closes #12851 from techaddict/SPARK-15072.

(cherry picked from commit ed6f3f8)
Signed-off-by: Andrew Or <[email protected]>
@techaddict techaddict deleted the SPARK-15072 branch May 6, 2016 12:00
asfgit pushed a commit that referenced this pull request May 12, 2016
…port in PySpark

## What changes were proposed in this pull request?
This is a followup of #12851
Remove `SparkSession.withHiveSupport` in PySpark and instead use `SparkSession.builder. enableHiveSupport`

## How was this patch tested?
Existing tests.

Author: Sandeep Singh <[email protected]>

Closes #13063 from techaddict/SPARK-15072-followup.
asfgit pushed a commit that referenced this pull request May 12, 2016
…port in PySpark

## What changes were proposed in this pull request?
This is a followup of #12851
Remove `SparkSession.withHiveSupport` in PySpark and instead use `SparkSession.builder. enableHiveSupport`

## How was this patch tested?
Existing tests.

Author: Sandeep Singh <[email protected]>

Closes #13063 from techaddict/SPARK-15072-followup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants