[GitHub] [kafka] jlprat commented on a change in pull request #11350: Scala3 migration

[GitBox <gi...@apache.org>]

jlprat commented on a change in pull request #11350:
URL: https://github.com/apache/kafka/pull/11350#discussion_r713423728



##########
File path: core/src/main/scala/kafka/controller/PartitionStateMachine.scala
##########
@@ -477,7 +477,7 @@ class ZkPartitionStateMachine(config: KafkaConfig,
     } else {
       val (logConfigs, failed) = zkClient.getLogConfigs(
         partitionsWithNoLiveInSyncReplicas.iterator.map { case (partition, _) => partition.topic }.toSet,
-        config.originals()
+        config.originals

Review comment:
       Changes like this are the parenthesis-less methods that should be called without parenthesis

##########
File path: core/src/test/java/kafka/testkit/KafkaClusterTestKit.java
##########
@@ -234,7 +234,7 @@ public KafkaClusterTestKit build() throws Exception {
                         Option.apply(threadNamePrefix),
                         JavaConverters.asScalaBuffer(Collections.<String>emptyList()).toSeq(),
                         connectFutureManager.future,
-                        Server.SUPPORTED_FEATURES()
+                        Server$.MODULE$.SUPPORTED_FEATURES()

Review comment:
       Changes like this are related to https://github.com/lampepfl/dotty/issues/13572

##########
File path: core/src/main/scala/kafka/admin/TopicCommand.scala
##########
@@ -22,7 +22,7 @@ import java.util.{Collections, Properties}
 import joptsimple._
 import kafka.common.AdminCommandFailedException
 import kafka.log.LogConfig
-import kafka.utils._
+import kafka.utils.{immutable => _, _}

Review comment:
       Changes like this are due to shadowing between `kafka.utils.immutable` and the `immutable` package in `scala.collections`.

##########
File path: core/src/main/scala/kafka/tools/ConsoleConsumer.scala
##########
@@ -27,7 +27,7 @@ import com.typesafe.scalalogging.LazyLogging
 import joptsimple._
 import kafka.utils.Implicits._
 import kafka.utils.{Exit, _}
-import org.apache.kafka.clients.consumer.{Consumer, ConsumerConfig, ConsumerRecord, KafkaConsumer}
+import org.apache.kafka.clients.consumer.{Consumer, ConsumerConfig => ClientConsumerConfig, ConsumerRecord, KafkaConsumer}

Review comment:
       This is done to avoid shadowing

##########
File path: core/src/main/scala/kafka/network/SocketServer.scala
##########
@@ -467,1251 +451,3 @@ object SocketServer {
 
   val ListenerReconfigurableConfigs = Set(KafkaConfig.MaxConnectionsProp, KafkaConfig.MaxConnectionCreationRateProp)
 }
-

Review comment:
       This is what I mentioned about splitting classes present in `SocketServer` into their own file

##########
File path: core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
##########
@@ -42,11 +42,11 @@ final class KafkaMetadataLog private (
   // Access to this object needs to be synchronized because it is used by the snapshotting thread to notify the
   // polling thread when snapshots are created. This object is also used to store any opened snapshot reader.
   snapshots: mutable.TreeMap[OffsetAndEpoch, Option[FileRawSnapshotReader]],
-  topicPartition: TopicPartition,
+  topicPartitionArg: TopicPartition,

Review comment:
       Changes like this are to avoid the shadowing between the parameter and the method

##########
File path: core/src/main/scala/kafka/log/LazyIndex.scala
##########
@@ -21,7 +21,7 @@ import java.io.File
 import java.nio.file.{Files, NoSuchFileException}
 import java.util.concurrent.locks.ReentrantLock
 
-import LazyIndex._
+

Review comment:
       This import was causing a cyclic problem in Scala3.

##########
File path: core/src/main/scala/kafka/utils/json/DecodeJson.scala
##########
@@ -85,13 +85,13 @@ object DecodeJson {
     else decodeJson.decodeEither(node).map(Some(_))
   }
 
-  implicit def decodeSeq[E, S[+T] <: Seq[E]](implicit decodeJson: DecodeJson[E], factory: Factory[E, S[E]]): DecodeJson[S[E]] = (node: JsonNode) => {
+  implicit def decodeSeq[E, S[E] <: Seq[E]](implicit decodeJson: DecodeJson[E], factory: Factory[E, S[E]]): DecodeJson[S[E]] = (node: JsonNode) => {

Review comment:
       Scala3 compiler is more strict with type definitions and the previous one wasn't really being satisfied

##########
File path: core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala
##########
@@ -2244,7 +2244,7 @@ class AuthorizerIntegrationTest extends BaseRequestTest {
   def testDescribeClusterClusterAuthorizedOperationsWithoutDescribeCluster(): Unit = {
     removeAllClientAcls()
 
-    for (version <- ApiKeys.DESCRIBE_CLUSTER.oldestVersion to ApiKeys.DESCRIBE_CLUSTER.latestVersion) {
+    for (version <- ApiKeys.DESCRIBE_CLUSTER.oldestVersion.toInt to ApiKeys.DESCRIBE_CLUSTER.latestVersion.toInt) {

Review comment:
       The `to` method is not present in `Short` type and Scala3 doesn't widen the type automatically

##########
File path: core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala
##########
@@ -481,8 +481,10 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
       // try a newCount which would be a decrease
       alterResult = client.createPartitions(Map(topic1 ->
         NewPartitions.increaseTo(1)).asJava, option)
-      
-      var e = assertThrows(classOf[ExecutionException], () => alterResult.values.get(topic1).get,
+      var e = assertThrows(classOf[ExecutionException], () => {
+        alterResult.values.get(topic1).get
+        ()
+      },

Review comment:
       This change is because of https://github.com/lampepfl/dotty/issues/13549




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org