Skip to content

Conversation

ttc0419
Copy link
Contributor

@ttc0419 ttc0419 commented Feb 10, 2023

According to the official document, SQLContext is deprecated since spark 2.0. Use SparkSession instead.

Notes:
This MR also removes CI for spark 2.4.8 since it does not support SparkSession.getActiveSession(). And according to versioning policy, spark 2.4.x is EOL and the last version was released in 5 years ago. There's little meaning to support it.

Closes: #423

@ttc0419
Copy link
Contributor Author

ttc0419 commented Feb 13, 2023

Tagging @WeichenXu123, since you reviewed the most recent MR.

@WeichenXu123
Copy link
Contributor

@ttc0419 Thank you! I will take a look soon.

@WeichenXu123
Copy link
Contributor

The trait GraphFrameTestSparkContext still has code protected override def _sqlContext: SQLContext = self.sqlContext, can we replace it with SparkSession ?

William Tang added 2 commits February 14, 2023 13:08
Spark 2.4.x does not support SparkSession.getActiveSession().  According to https://spark.apache.org/versioning-policy.html, spark 2.4.x is EOL and the last version was released 5 years ago.  There's little meaning to support it.
@ttc0419
Copy link
Contributor Author

ttc0419 commented Feb 14, 2023

The trait GraphFrameTestSparkContext still has code protected override def _sqlContext: SQLContext = self.sqlContext, can we replace it with SparkSession ?

I'm not familiar with Scala, probably need someone else and another MR to do that.

@ttc0419 ttc0419 requested a review from WeichenXu123 February 14, 2023 07:01
@WeichenXu123
Copy link
Contributor

The trait GraphFrameTestSparkContext still has code protected override def _sqlContext: SQLContext = self.sqlContext, can we replace it with SparkSession ?

I'm not familiar with Scala, probably need someone else and another MR to do that.

We can do it in follow-up PR.

@ttc0419
Copy link
Contributor Author

ttc0419 commented Feb 14, 2023

@WeichenXu123 Anything else need to be done before merging this?

Copy link
Contributor

@WeichenXu123 WeichenXu123 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Merging #424 (59f5bd0) into master (100fb01) will decrease coverage by 0.19%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #424      +/-   ##
==========================================
- Coverage   91.37%   91.19%   -0.19%     
==========================================
  Files          18       18              
  Lines         870      829      -41     
  Branches       53       49       -4     
==========================================
- Hits          795      756      -39     
+ Misses         75       73       -2     
Impacted Files Coverage Δ
src/main/scala/org/graphframes/Logging.scala 75.00% <0.00%> (-5.00%) ⬇️
src/main/scala/org/graphframes/GraphFrame.scala 87.11% <0.00%> (-2.07%) ⬇️
...c/main/scala/org/graphframes/examples/Graphs.scala 100.00% <0.00%> (ø)
...in/scala/org/graphframes/GraphFramePythonAPI.scala 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ttc0419
Copy link
Contributor Author

ttc0419 commented Feb 15, 2023

@WeichenXu123 Do we need another reviewer to merge this?

@WeichenXu123
Copy link
Contributor

@WeichenXu123 Do we need another reviewer to merge this?

I can merge it.

@WeichenXu123 WeichenXu123 merged commit 1e1dbd3 into graphframes:master Feb 15, 2023
@ttc0419 ttc0419 deleted the spark-session branch February 15, 2023 12:12
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.

Couple of warnings for python
3 participants