You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "ijuma (via GitHub)" <gi...@apache.org> on 2023/06/10 16:21:05 UTC

[GitHub] [kafka] ijuma opened a new pull request, #13840: MINOR: Upgrade Scala for Java 20/21 support

ijuma opened a new pull request, #13840:
URL: https://github.com/apache/kafka/pull/13840

   Upgrade to Scala 2.13.11 and Scala 2.12.18.
   
   Scala 2.13 release notes:
   * https://github.com/scala/scala/releases/tag/v2.13.11
   
   Scala 2.12 release notes:
   * https://github.com/scala/scala/releases/tag/v2.12.16
   * https://github.com/scala/scala/releases/tag/v2.12.17
   * https://github.com/scala/scala/releases/tag/v2.12.18
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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

[GitHub] [kafka] ijuma commented on pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #13840:
URL: https://github.com/apache/kafka/pull/13840#issuecomment-1599220710

   JDK 11 build passed, there were two unrelated failures outside of that:
   
   > Build / JDK 17 and Scala 2.13 / testMaxConnectionsPerIp() – kafka.network.SocketServerTest
   15s
   Build / JDK 8 and Scala 2.12 / testBalancePartitionLeaders() – org.apache.kafka.controller.QuorumControllerTest


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

[GitHub] [kafka] jolshan commented on a diff in pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on code in PR #13840:
URL: https://github.com/apache/kafka/pull/13840#discussion_r1227308577


##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2499,15 +2499,16 @@ class ReplicaManagerTest {
                                                          time: Time,
                                                          threadNamePrefix: Option[String],
                                                          replicationQuotaManager: ReplicationQuotaManager): ReplicaFetcherManager = {
+        val rm = this

Review Comment:
   do we want to replace "this" in the line below with rm? 



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

[GitHub] [kafka] ijuma merged pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma merged PR #13840:
URL: https://github.com/apache/kafka/pull/13840


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

[GitHub] [kafka] jlprat commented on pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "jlprat (via GitHub)" <gi...@apache.org>.
jlprat commented on PR #13840:
URL: https://github.com/apache/kafka/pull/13840#issuecomment-1586853893

   Hi @ijuma from the release notes:
   
   > Known issues
   > 
   > A few regressions have been discovered:
   > 
   > Exhaustivity checker emits spurious warning when matching on Java enum type (2.13.11 regression) (https://github.com/scala/bug/issues/12800)
   > Duplicated `@Deprecated` annotations when extending Java interface with deprecated default method cause java.lang.annotation.AnnotationFormatError when accessed via Java reflection (2.13.11 regression) (https://github.com/scala/bug/issues/12799)
   > 
   > We'll address these in Scala 2.13.12.
   
   I don't think we are hitting those, but I just wanted to make sure we are aware of these before merging.


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

[GitHub] [kafka] ijuma commented on pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #13840:
URL: https://github.com/apache/kafka/pull/13840#issuecomment-1585822711

   JDK 8 and 17 builds passed, 11 is queued.


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

[GitHub] [kafka] ijuma commented on a diff in pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #13840:
URL: https://github.com/apache/kafka/pull/13840#discussion_r1234197390


##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2499,15 +2499,16 @@ class ReplicaManagerTest {
                                                          time: Time,
                                                          threadNamePrefix: Option[String],
                                                          replicationQuotaManager: ReplicationQuotaManager): ReplicaFetcherManager = {
+        val rm = this

Review Comment:
   It doesn't make a difference since it's before the scope of the inner class of `ReplicaFetcherManager` is created, but I made this change for clarity.



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

[GitHub] [kafka] ijuma commented on pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #13840:
URL: https://github.com/apache/kafka/pull/13840#issuecomment-1597374541

   Thanks for the reviews. A couple of responses.
   
   > I don't think we are hitting those, but I just wanted to make sure we are aware of these before merging.
   
   Yes, I saw that note but didn't seem like a blocker for us.
   
   > I found a shorter way to make the test compile (and run successfully). One needs to type the replicaManager variable in line 2481:
   
   Interesting - I tried that originally and it didn't work for me. In any case, the fact that we were referring to `this` inside a new anonymous class to refer to the outer `ReplicaManager` anonymous class is confusing. So, I think the change I made makes things clearer in the end.
   


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

[GitHub] [kafka] jlprat commented on pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "jlprat (via GitHub)" <gi...@apache.org>.
jlprat commented on PR #13840:
URL: https://github.com/apache/kafka/pull/13840#issuecomment-1597378453

   > So, I think the change I made makes things clearer in the end.
   
   I agree. Thanks @ijuma 


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

[GitHub] [kafka] jlprat commented on pull request #13840: MINOR: Upgrade Scala for Java 20/21 support

Posted by "jlprat (via GitHub)" <gi...@apache.org>.
jlprat commented on PR #13840:
URL: https://github.com/apache/kafka/pull/13840#issuecomment-1586983158

   I triggered the build again


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