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

Drop Support of library build with Scala version 2.12 #155

Closed
novakov-alexey opened this issue Oct 6, 2024 · 7 comments · Fixed by #176
Closed

Drop Support of library build with Scala version 2.12 #155

novakov-alexey opened this issue Oct 6, 2024 · 7 comments · Fixed by #176
Assignees
Labels
good first issue Good for newcomers

Comments

@novakov-alexey
Copy link
Collaborator

novakov-alexey commented Oct 6, 2024

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 novakov-alexey added the good first issue Good for newcomers label Oct 14, 2024
@novakov-alexey
Copy link
Collaborator Author

to be done in the next release.

@arnaud-daroussin
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link

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:

  • migrate to flink-scala-api on the 1.X version and staying in Scala 2.12, handling all changes necessary from Flink's Scala API;
  • then migrate to the latest 2.X version of flink-scala-api and Scala 2.13.

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
Copy link
Collaborator Author

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
Copy link

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
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants