Scala3 builds #1603
Scala3 builds #1603
Conversation
sebastian-alfers
force-pushed
the
scala3-builds
branch
from
95dd770 to
31d3c25
Compare
| @@ -187,8 +187,6 @@ private[kafka] final class CommittableOffsetBatchImpl( | |||
| val metadata = newOffset match { | |||
| case offset: CommittableOffsetMetadata => | |||
| offset.metadata | |||
| case _ => | |||
There was a problem hiding this comment.
CommittableOffsetMetadata is the only subtype of a sealed trait - so it should be ok to be removed as scala3 complained about it.
| /** | ||
| * Provide access to `apply` which Scala3 makes private. | ||
| */ | ||
| def create[K, V]( |
There was a problem hiding this comment.
why is this create and Result has apply?
There was a problem hiding this comment.
Do I understand it correctly that Scala 3 doesn't add apply if the constructor is private? Can we still name it apply to be source compatible, or is that in conflict with Scala 2?
There was a problem hiding this comment.
I think Scala3 makes the constructor private, which is why my suggestion is to provide apply through the companion object which has access to private methods (the constructor).
The create approach also works, but we should stick to apply. I missed that and made it consistent now.
| @@ -57,7 +59,7 @@ private[kafka] trait DeferredProducer[K, V] { | |||
| } | |||
|
|
|||
| final protected def resolveProducer(settings: ProducerSettings[K, V]): Unit = { | |||
| val producerFuture = settings.createKafkaProducerAsync()(materializer.executionContext) | |||
| val producerFuture = settings.createKafkaProducerAsync()(interpreter.materializer.executionContext) | |||
There was a problem hiding this comment.
Because materializer is protected, I presume.
That is not great since interpreter is private[akka] internal API. Yes, it can be accessed from this akka package but we want to avoid using internal apis like that.
Would it be possible to change trait DeferredProducer to
abstract class DeferredProducer extends GraphStageLogic with StageIdLogging
There was a problem hiding this comment.
Seems to work 👍
| @@ -760,6 +759,8 @@ import scala.util.control.NonFatal | |||
|
|
|||
| override def onPartitionsRevoked(partitions: java.util.Collection[TopicPartition]): Unit = () | |||
|
|
|||
| override def onPartitionsLost(partitions: java.util.Collection[TopicPartition]): Unit = () | |||
There was a problem hiding this comment.
ah, Scala 3 doesn't understand default method from Java interface
From ConsumerRebalanceListener:
default void onPartitionsLost(Collection<TopicPartition> partitions) {
onPartitionsRevoked(partitions);
}
@SethTisue do you think this should be reported upstreams or is it intentional?
There was a problem hiding this comment.
a few lines up, on line 751, you have
override def onPartitionsLost(partitions: java.util.Collection[TopicPartition]): Unit
I think that's why the Scala 3 compiler isn't seeing that the Java interface in question has a default implementation for this method — it's been overridden by this abstract method.
If I remove line 751 and line 762, it compiles.
There was a problem hiding this comment.
Incidentally one of the first things I tried here was Scala 3.2.2 instead of 3.1.3. It turned out not to make any difference in this case, but regardless, I'd recommend moving to 3.2.2 if you can, as bugfixes are always landing. Also 3.3.0 will be out soon (they're at 3.3.0-RC3).
There was a problem hiding this comment.
Thanks for looking into it @SethTisue
I think we should stick to the lower 3.1 version since we want to support as many versions as possible (if the cost isn't too high). Scala 3.x is backwards compatible but not forwards compatible so if we bump to lates we would exclude users of earlier versions.
There was a problem hiding this comment.
okay, but lots of projects are taking the opposite view, that it's fine and normal for users to need to upgrade their Scala 3 minor version when upgrading a dependency. that's also the view that the Scala Center is encouraging library maintainers to take. and we've already merged the 3.2 upgrade even in foundational libraries such as scala-xml and scala-collection-compat.
Scala 3.3 will be the first "long term support" version of Scala 3. you might consider adopting your more conservative view once 3.3 is available, but take the more liberal view until then. 3.1 isn't getting bugfixes anymore, and 3.2 isn't either, now that 3.3 is imminent.
There was a problem hiding this comment.
ok, good to know. We should anyway try to use the same version in all Akka projects as much as possible, so such change would not start here.
There was a problem hiding this comment.
Do you want to take the opportunity and bump the scala version in all projects before the next release?
sebastian-alfers
force-pushed
the
scala3-builds
branch
from
fcb7b1a to
f4f84e9
Compare
sebastian-alfers
force-pushed
the
scala3-builds
branch
from
f4f84e9 to
f210404
Compare
No description provided.