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.

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

Scala2.13 + Spark v3.3 #199

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Scala2.13 + Spark v3.3 #199

wants to merge 8 commits into from

Conversation

requaos
Copy link

@requaos requaos commented Feb 13, 2023

No description provided.

@lresende
Copy link
Member

Thanks for this @requaos , let me start some discussion on how to manage this with regard supporting for old Spark releases.

@requaos
Copy link
Author

requaos commented Feb 26, 2023

I would appreciate some guidance there.

@lresende
Copy link
Member

So it looks like the tests didn't get updated. I spent a little time on it and got the scala tests updated, but still seeing some issues related to Mockito, let me try to fix those and then I will push a commit to your pr.

@lresende
Copy link
Member

I finally found some time to spend on this PR, it's not completely passing all tests, but we are down to a hand full ones to fix.

@lresende
Copy link
Member

For testing it locally, instead of just make dist try make clean dist test

@ywskycn
Copy link
Contributor

ywskycn commented Jun 30, 2023

@lresende any update on this change?

@pan3793
Copy link
Member

pan3793 commented Aug 4, 2023

@requaos @lresende would you mind me taking over this Spark & Scala upgrading work? I would like to split this into some small tasks to make it easy to review if you don't mind.

Spark 3.0 ~ 3.5 uses Scala 2.12 by default, it rarely uses a non-default Scala version in production, so I would like to upgrade the Spark first.

BTW, how many versions of Spark does this project expect to support at the same time? I don't find a doc or GitHub Action workflow to describe that.

@requaos
Copy link
Author

requaos commented Aug 7, 2023

@pan3793 This effort is poised to provide Scala 2.13 notebook kernel functionality. This overcomes a direct need in the field to have a hands-on Scala 2.13-based repl, and/or Jupyter, experience. The effort here will build on up-to-date Spark version support. Please let me know what I can do to prepare this PR into the singular context of Scala 2.13 support.

@pan3793
Copy link
Member

pan3793 commented Aug 8, 2023

@requaos thanks for sharing the background, I skimmed through the change, the most of the efforts are spent on dependencies upgrading, and unfortunately, some of them introduce breaking changes so a lot of code need to be adjusted. e.g. Spark, scalatest, mockito, coursier. It is worth upgrading them in independent PRs.

And I believe some of the dependencies upgradings are not required, e.g. sbt, akka. Especially, akka changed the license from Apache V2 to BSL v1.1 in 2.7, the latter is not allowed by Apache projects.

https://www.thestack.technology/akka-license-change-lightbend-opensource/
https://apache.org/legal/resolved.html#category-x

@pan3793
Copy link
Member

pan3793 commented Aug 8, 2023

@requaos May I ask a little more about the requirements of Scala 2.13?

This is a Spark ecosystem project, most dependencies are intended to align with Spark, as I mentioned above

Spark 3.0 ~ 3.5 uses Scala 2.12 by default, it rarely uses a non-default Scala version in production

So I suppose changing it to Scala 2.13 and dropping 2.12 looks aggressive. It makes more sense if we can support both Scala 2.12 and 2.13 and use 2.12 in default, but this means introducing some refactoring of the building system.

@requaos
Copy link
Author

requaos commented Aug 8, 2023

@pan3793 Yes, this is a larger effort than initially planned and I have not been as considerate of license for dependency updates. Planning forward to support both Scala 2.12 and 2.13, with a default preference for 2.12, will be most optimal.
I can accommodate upstream changes in my fork to assist in this effort. As you see fit to divide this labor, I will gladly reduce the scope of my PR to reflect an incremental task to work more gradually towards our shared goal.

@pan3793
Copy link
Member

pan3793 commented Aug 8, 2023

@requaos Thanks, as part of this PR, I think you can upgrade coursier to the latest 2.x version in a separate PR first, to gain support for both Scala 2.12 and 2.13.

I would like to take a look at scalatest/mockito stuff after #201 gets merged.

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