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

SPARKC-686 Port to Scala 2.13 #1349

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marquiswang
Copy link

Description

How did the Spark Cassandra Connector Work or Not Work Before this Patch

Spark Cassandra Connector did not work with Scala 2.13 before this patch.

General Design of the patch

No real API changes. In general, just migrating from scala.collection.JavaConversions to scala.jdk.CollectionConverters. Some usages of Seq needed to be migrated to immutable.Seq.

I apologize ahead of time if there's anything that is suboptimal here - I'm not a Scala developer so I'm not entirely familiar with best practices and how to do this migration. However, I think this stuff was all pretty straightforward with these two migrations.

I'm also not sure exactly how you would want to release this and how you'd like to branch it. Please let me know if you'd like me to do a PR to a different branch or something.

How Has This Been Tested?

All tests pass when run on my machine with sbt/sbt test.

Checklist:

  • I have a ticket in the OSS JIRA
  • I have performed a self-review of my own code
  • Locally all tests pass (make sure tests fail without your patch)

@marquiswang marquiswang changed the title Port to Scala 2.13 SPARKC-686 Port to Scala 2.13 Jun 22, 2022
@msmygit
Copy link
Collaborator

msmygit commented Jun 22, 2022

@nik-kashi
Copy link

Any update on this pull?

build.sbt Outdated
lazy val scala212 = "2.12.11"
lazy val supportedScalaVersions = List(scala212)
lazy val scala213 = "2.13.8"
lazy val supportedScalaVersions = List(scala213)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We would like to support 2.12 and 2.13. Could you please adjust the build to support both versions?
Additionally, could you modify github actions definition to run for 2.13 as well?

Copy link
Author

Choose a reason for hiding this comment

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

Done, I rebased my change on top of master and added 2.12 to here and GitHub actions.

I am not a Scala developer normally, nor do I work with Github actions, so I have no idea if I did it correctly, so apologies if it doesn't actually work. sbt test seems to pass for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! We definitely need https://github.com/scala/scala-collection-compat to make this work as the adjusted code isn't compatible with 2.12. You may try it yourself by typing sbt/sbt ++2.12.11 test in your cmd.

Unfortunately even with scala-collection-compat we have problems with scala.collection.mutable.Builder used om.datastax.spark.connector.types.CanBuildFrom. The api is different, we might want to substitute the builder with a custom class. Would you like to take a look? If not I will give a try later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

scala compat: marquiswang#1

Copy link
Author

Choose a reason for hiding this comment

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

Many other Spark libraries release two binaries for the different Scala versions. Maybe we just want to do that?

https://search.maven.org/search?q=a:spark-launcher_2.1*

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's the goal here. We want to keep a single code base tho.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I had assumed those projects just have a few slightly different branches.

I'm not sure if I'm going to be able to deal with the compatibility issues. Probably better for you to have a look.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please sign https://cla.datastax.com/ ?

Copy link
Author

Choose a reason for hiding this comment

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

I believe I've already signed it! It shows as "Completed".

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll work on Scala 2.13 support in the background and in the meantime I'll publish Spark 3.3 with Scala 2.12 support. Thank you for this contribution!

Major changes:
* Migrate from scala.collection.JavaConversions to
  scala.jdk.CollectionConverters
* Migrate some uses of Seq to immutable.Seq
@jesperancinha
Copy link

Hi there, when do you reckon this release will become available? I was looking to open an issue for you and I came across this one. In the only repo I'm using in this library, everything is failing for a while now and I come across more updates that I cannot do anymore: https://github.com/jesperancinha/bridges-surveillance-story. An example of an indirect limitation is that the Glassfish containers that my modules are implicitly using have suffered an update and now they are Jakarta containers and not Javax containers. And if I update this to use Javax then I get other conflicts. I'm also getting serialization issues but long story short, I need the new spark cassandra connector. Please let me know. Thanks in advance!

@whahmedibrahim
Copy link

Hi there. Does anybody know of an estimated timespan for when a Scala 2.13 version of the connector will be released? Thank you in advance

@cheapsolutionarchitect
Copy link

No real API changes. In general, just migrating from scala.collection.JavaConversions to scala.jdk.CollectionConverters. Some usages of Seq needed to be migrated to immutable.Seq.

Hm, is this really advisable? There maybe memory and performance impacts due to the use of immutable collections.

@jesperancinha
Copy link

Hi there! How is the progress going on this issue? I just want to know a bit about how this is going. I think I might be looking for alternatives soon. My whole project can't get updated as it is, to Scala 2.13 solely because of this issue, so I definitely need to think about other alternatives soon.

@SamTheisens
Copy link
Contributor

Another attempt to finish this work has been made here #1361

@jesperancinha
Copy link

I can' really follow that well the work you are doing because I thought this branch was going to be the one to determine the next version supporting scala 2.13. I noticed that a release was already made at the 3rd quarter of last year. I noticed it just now. I updated my project and after struggling a bit with it everythng works! So thank YOU so much for this! Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants