search

flink-extended / flink-scala-api

Flink Scala API is a thin wrapper on top of Flink Java API which support Scala Types for serialisation as well the latest Scala version
Apache License 2.0
68 stars 14 forks source link

Drop Support of library build with Scala version 2.12 #155

Open novakov-alexey opened 1 month ago

novakov-alexey commented 1 month ago

In order to reduce time for the release process and get rid of obsolete code related to Scala 2.12.

Please share your feedback if build for Scala 2.12 is still needed.

If no meaningful reasons shared to keep Scala 2.12, then it will be dropped in one month from now.

novakov-alexey commented 1 week ago

to be done in the next release.

arnaud-daroussin commented 1 week ago

Hi @novakov-alexey, There is a very good reason to keep Scala 2.12: to ease smooth transition between flink-streaming-scala stuck in 2.12.7 and flink-streaming-java + flink-scala-api, we have chosen to take one step with Scala 2.12.20. It's already a big migration for us, having to upgrade to Scala 2.13 at the same time would make the migration even harder. I suggest to keep the Scala 2.12 support for Flink 1.x and remove it for Flink 2.x. Thanks

novakov-alexey commented 6 days ago

Hi @arnaud-daroussin ,

Thank you for your message.

I recommend you to transition to 2.13 in the same step as well. The 2.12 is very old version of Scala, which does not makes sense to continue with it anymore.

Maybe you could try and report back whether upgrade to 2.13 brings any issue for you? We could keep this issue on hold for one more month.

gaelrenoux-datadome commented 4 days ago

Hey @novakov-alexey,

Another idea that's probably simpler to maintain: you could just document the latest version that supports Scala 2.12. For example, assuming that dropping Scala 2.12 support is a new major version (so it's 2.0.0), the migration path would be:

It's just some documentation to maintain, and maybe occasionally fix a critical bug on the old branch.

On the other hand, migrating both to flink-scala-api and to Scala 2.13 can be a bit annoying. The collection overhaul in Scala 2.13 can require quite a bit of changes, depending of how you used them (for example if you've created utility method that use the CanBuildFrom construct). It's obviously possible, but it makes for huge PRs and unpleasant reviews.

novakov-alexey commented 3 days ago

Hi @gaelrenoux-datadome

Thank you for your input.

I actually thought to continue to use version 1.x even if Scala 2.12 is dropped. When we say Scala 2.12 is dropped, we mean that this project won't longer publish flink-scala-api_2.12 JAR file to the Maven central. However, public API will be still the same. As a library user you won't need to adjust your code much, perhaps only those things you mentioned about Scala collections. However, you do not use much Scala collections in Flink general. What for?

I am afraid that maintaining two major versions in parallel: 1.x with Scala 2.12 2.x with Scala 2.13, Scala 3

would create even more work for me. So far I am pretty much alone who maintains this library.

So far what I am hearing from you: it is kind of more work to do when switching to Scala 2.13, then staying on it. That means, if you are already using this library with Scala 2.12, then you do not need to anything to receive new updates of this library, but if you loose support of Scala 2.12 build, then you have to first migrate to 2.13. I am still tryng to understand what are the main blockers to migrate for you from ancient old Scala 2.12 to at least to 2.13. I am not even saying to Scala 3.x.

As for me, migration of your Flink jobs from Scala 2.12 to Scala 2.13 should be easy. I need more arguments. If I won't hear convincing arguments I will drop Scala 2.12 within next month or so.

gaelrenoux-datadome commented 23 hours ago

Sorry, I wasn't clear: I'm completely fine with the 1.X (or 1.2.X) receiving no more updates at all. I just think that it's worth it to keep in the documentation a line saying "Scala 2.12 is only supported up to version 1.2.0". That way, users have the information they need to migrate that way if they need to. Your proposed PR completely removes all traces of Scala 2.12 in the README.

As far the Scala 2.12 => 2.13 migration: we have a few internal utilites, adding basic functions like doubly mapping (over a collection of collections) or zipping with the result of a function. All of that typically applies to Traversable (which doesn't exist anymore in 2.13), and uses CanBuildFrom to get the output collection type be the same as the input one. Is it hard to migrate ? Not really, it's one or two hours at most. However, it's still a bunch of classes to change, that will have to end up in the same PR as the Flink API migration (or the PR won't compile). That results in a big PR that concerns itself with completely different topics, so it's not straightforward to review.

Is it a dealbreaker? No. We would have managed just fine, or we would have looked in the project history to figure out if there's a version that we can use while staying in Scala 2.12. It's just that our use case doesn't seem that rare, since anyone migrating from the base Flink Scala API to your library would be in Scala 2.12, and the cost of keeping a line in the README seems pretty low to me. But then, I'm not the one maintaining this library, and we'll still use it no matter what :-)

novakov-alexey commented 20 hours ago

Thanks for explanation, I appreciate it. This helps indeed. Adding a note to documentation about release version which could be used with Scala 2.12 is fine to me.