Thanks Josep, I've applied for a JIRA account and addressed your review comments.
Am Do., 8. Feb. 2024 um 09:19 Uhr schrieb Josep Prat <josep.p...@aiven.io.invalid>: > > Hi Matthias, > > I think for this particular case it would be worth creating a JIRA ticket > for this as it's a new "feature". > Regarding the change itself, I think we need to clarify how the release > process would work. Right now, the script `gradlewAll` is used (which > basically runs the build with Scala version 2.12 and 2.13). If I understand > your changes correctly, we would need to run the build 3 times: > - 1 with property scalaVersion 2.12 > - 1 with scalaVersion 2.13 and streamsScalaVersion 2.13 > - 1 with scalaVersion 2.13 and streamsScalaVersion 3.1 > > I think we should document this and discuss when to have this feature. If I > remember correctly from when I tried to update Kafka to Scala 3, the idea > was to push this to a Kafka 4.0 version because we didn't want to maintain > more than 2 Scala versions at the same time. I would encourage if not > having a KIP, at least open up a [DISCUSS] thread to clarify some of these > points. > > I'll add some feedback on the PR itself regarding the changes. > > Best, > > On Thu, Feb 8, 2024 at 1:57 AM Matthias Berndt <matthias.ber...@ttmzero.com> > wrote: > > > Hi Matthias J., Hi Lucas, Hi Josep, > > > > Thank you for your encouraging responses regarding a Scala 3 port of > > Kafka-Streams-Scala, and apologies for the late response from my side. > > I have now created a PR to port Kafka-Streams-Scala to Scala 3 (while > > retaining support for 2.13 and 2.12). Almost no changes to the code > > were required and the tests also pass. Please take a look and let me > > know what you think :-) > > https://github.com/apache/kafka/pull/15338 > > > > All the best > > Matthias > > > > Am Do., 1. Feb. 2024 um 16:35 Uhr schrieb Josep Prat > > <josep.p...@aiven.io.invalid>: > > > > > > Hi, > > > > > > For reference, prior work on this: > > > https://github.com/apache/kafka/pull/11350 > > > https://github.com/apache/kafka/pull/11432 > > > > > > Best, > > > > > > On Thu, Feb 1, 2024, 15:55 Lucas Brutschy <lbruts...@confluent.io > > .invalid> > > > wrote: > > > > > > > Hi Matthiases, > > > > > > > > I know Scala 2 fairly well, so I'd be happy to review changes that add > > > > Scala 3 support. However, as Matthias S. said, it has to be driven by > > > > people who use Scala day-to-day, since I believe most Kafka Streams > > > > committers are working with Java. > > > > > > > > Rewriting the tests to not use EmbeddedKafkaCluster seems like a large > > > > undertaking, so option 1 is the first thing we should explore. > > > > > > > > I don't have any experience with Scala 3 migration topics, but on the > > > > Scala website it says > > > > > The first piece of good news is that the Scala 3 compiler is able to > > > > read the Scala 2.13 Pickle format and thus it can type check code that > > > > depends on modules or libraries compiled with Scala 2.13. > > > > > One notable example is the Scala 2.13 library. We have indeed decided > > > > that the Scala 2.13 library is the official standard library for Scala > > 3. > > > > So wouldn't that mean that we are safe in terms of standard library > > > > upgrades if we use core_2.13 in the tests? > > > > > > > > Cheers, > > > > Lucas > > > > > > > > > > > > On Wed, Jan 31, 2024 at 9:20 PM Matthias J. Sax <mj...@apache.org> > > wrote: > > > > > > > > > > Thanks for raising this. The `kafka-streams-scala` module seems to > > be an > > > > > important feature for Kafka Streams and I am generally in favor of > > your > > > > > proposal to add Scala 3 support. However, I am personally no Scala > > > > > person and it sounds like quite some overhead. > > > > > > > > > > If you are willing to drive and own this initiative happy to support > > you > > > > > to the extend I can. > > > > > > > > > > About the concrete proposal: my understanding is that :core will move > > > > > off Scala long-term (not 100% sure what the timeline is, but new > > modules > > > > > are written in Java only). Thus, down the road the compatibility > > issue > > > > > would go away naturally, but it's unclear when. > > > > > > > > > > Thus, if we can test kafak-stream-scala_3 with core_2.13 it seems we > > > > > could add support for Scala 3 now, taking a risk that it might break > > in > > > > > the future assume that the migration off Scala from core is not fast > > > > enough. > > > > > > > > > > For proposal (2), I don't think that it would be easily possible for > > > > > unit/integration tests. We could fall back to system tests though, > > but > > > > > they would be much more heavy weight of course. > > > > > > > > > > Might be good to hear from others. We might actually also want to do > > a > > > > > KIP for this? > > > > > > > > > > > > > > > -Matthias > > > > > > > > > > On 1/20/24 10:34 AM, Matthias Berndt wrote: > > > > > > Hey there, > > > > > > > > > > > > I'd like to discuss a Scala 3 port of the kafka-streams-scala > > library. > > > > > > Currently, the build system is set up such that kafka-streams-scala > > > > > > and core (i. e. kafka itself) are compiled with the same Scala > > > > > > compiler versions. This is not an optimal situation because it > > means > > > > > > that a Scala 3 release of kafka-streams-scala cannot happen > > > > > > independently of kafka itself. I think this should be changed > > > > > > > > > > > > The production codebase of scala-streams-kafka actually compiles > > just > > > > > > fine on Scala 3.3.1 with two lines of trivial syntax changes. The > > > > > > problem is with the tests. These use the `EmbeddedKafkaCluster` > > class, > > > > > > which means that kafka is pulled into the classpath, potentially > > > > > > leading to binary compatibility issues. > > > > > > I can see several approaches to fixing this: > > > > > > > > > > > > 1. Run the kafka-streams-scala tests using the compatible version > > of > > > > > > :core if one is available. Currently, this means that everything > > can > > > > > > be tested (test kafka-streams-scala_2.12 using core_2.12, > > > > > > kafka-streams-scala_2.13 using core_2.13 and kafka-streams-scala_3 > > > > > > using core_2.13, as these should be compatible), but when a new > > > > > > scala-library version is released that is no longer compatible with > > > > > > 2.13, we won't be able to test that. > > > > > > 2. Rewrite the tests to run without EmbeddedKafkaCluster, instead > > > > > > running the test cluster in a separate JVM or perhaps even a > > > > > > container. > > > > > > > > > > > > I'd be willing to get my hands dirty working on this, but before I > > > > > > start I'd like to get some feedback from the Kafka team regarding > > the > > > > > > approaches outlined above. > > > > > > > > > > > > All the best > > > > > > Matthias Berndt > > > > > > > KJosep Prat > > > Open Source Engineering Director, aivenjosep.p...@aiven.io | > > > +491715557497 | aiven.io > > > Aiven Deutschland GmbH > > > Alexanderufer 3-7, 10117 Berlin > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > > Amtsgericht Charlottenburg, HRB 209739 B > > > > > -- > [image: Aiven] <https://www.aiven.io> > > *Josep Prat* > Open Source Engineering Director, *Aiven* > josep.p...@aiven.io | +491715557497 > aiven.io <https://www.aiven.io> | <https://www.facebook.com/aivencloud> > <https://www.linkedin.com/company/aiven/> <https://twitter.com/aiven_io> > *Aiven Deutschland GmbH* > Alexanderufer 3-7, 10117 Berlin > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > Amtsgericht Charlottenburg, HRB 209739 B
