You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/11/23 15:46:38 UTC
[GitHub] [kafka] ijuma opened a new pull request #9643: MINOR: Upgrade to Scala 2.13.4 and 2.12.13
ijuma opened a new pull request #9643:
URL: https://github.com/apache/kafka/pull/9643
Scala 2.12.13 includes general bug fixes and performance improvements
to the collection libraries and the compiler backported from Scala 2.13.x.
Scala 2.13.4 restores default global `ExecutionContext` to 2.12 behavior
(to fix a perf regression in some use cases) and improves pattern matching
(especially exhaustiveness checking). Most of the changes are related
to the latter as I have enabled the newly introduced `-Xlint:strict-unsealed-patmat`.
More details on the code changes:
* Don't swallow exception in `ReassignPartitionsCommand.topicDescriptionFutureToState`.
* `RequestChannel.Response` should be `sealed`.
* Introduce sealed ClientQuotaManager.BaseUserEntity to avoid false positive
exhaustiveness warning.
* Handle a number of cases where pattern matches were not exhaustive:
either by marking them with @unchecked or by adding a catch-all clause.
* Workaround scalac bug related to exhaustiveness warnings in ZooKeeperClient
* Remove warning suppression annotations related to the optimizer that are no
longer needed in ConsumerGroupCommand and AclAuthorizer.
* Use `forKeyValue` in `AclAuthorizer.acls` as the scala bug preventing us from
using it seems to be fixed.
Full release notes:
https://github.com/scala/scala/releases/tag/v2.13.4
https://github.com/scala/scala/releases/tag/v2.12.12
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9643: MINOR: Upgrade to Scala 2.13.4 and 2.12.13
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9643:
URL: https://github.com/apache/kafka/pull/9643#discussion_r529043449
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -2418,13 +2418,11 @@ class Log(@volatile private var _dir: File,
info(s"Replacing overflowed segment $segment with split segments $newSegments")
replaceSegments(newSegments.toList, List(segment))
newSegments.toList
- } catch {
- case e: Exception =>
- newSegments.foreach { splitSegment =>
- splitSegment.close()
- splitSegment.deleteIfExists()
- }
- throw e
+ } finally {
+ newSegments.foreach { splitSegment =>
+ splitSegment.close()
+ splitSegment.deleteIfExists()
+ }
Review comment:
@dhruvilshah3 Is there a reason why we had a try/catch with a rethrow instead of using `finally`? spotBugs complained about it and hence the question.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #9643: MINOR: Upgrade to Scala 2.13.4
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9643:
URL: https://github.com/apache/kafka/pull/9643#issuecomment-733156772
Tests passed, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #9643: MINOR: Upgrade to Scala 2.13.4
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9643:
URL: https://github.com/apache/kafka/pull/9643#issuecomment-733008676
I reverted the Scala 2.12.13 change, looks like this hasn't been published to Maven yet.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9643: MINOR: Upgrade to Scala 2.13.4 and 2.12.13
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9643:
URL: https://github.com/apache/kafka/pull/9643#discussion_r529043449
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -2418,13 +2418,11 @@ class Log(@volatile private var _dir: File,
info(s"Replacing overflowed segment $segment with split segments $newSegments")
replaceSegments(newSegments.toList, List(segment))
newSegments.toList
- } catch {
- case e: Exception =>
- newSegments.foreach { splitSegment =>
- splitSegment.close()
- splitSegment.deleteIfExists()
- }
- throw e
+ } finally {
+ newSegments.foreach { splitSegment =>
+ splitSegment.close()
+ splitSegment.deleteIfExists()
+ }
Review comment:
@dhruvilshah3 Is there a reason why we had a try/catch with a rethrow instead of using `finally`? spotBugs complained about it and hence the question.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9643: MINOR: Upgrade to Scala 2.13.4
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9643:
URL: https://github.com/apache/kafka/pull/9643#discussion_r529585355
##########
File path: core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala
##########
@@ -151,21 +149,22 @@ object ConsumerGroupCommand extends Logging {
private[admin] case class CsvUtils() {
val mapper = new CsvMapper with ScalaObjectMapper
mapper.registerModule(DefaultScalaModule)
- def readerFor[T <: CsvRecord: ClassTag] = {
+ def readerFor[T <: CsvRecord : ClassTag] = {
val schema = getSchema[T]
val clazz = implicitly[ClassTag[T]].runtimeClass
mapper.readerFor(clazz).`with`(schema)
}
- def writerFor[T <: CsvRecord: ClassTag] = {
+ def writerFor[T <: CsvRecord : ClassTag] = {
val schema = getSchema[T]
val clazz = implicitly[ClassTag[T]].runtimeClass
mapper.writerFor(clazz).`with`(schema)
}
- private def getSchema[T <: CsvRecord: ClassTag] = {
+ private def getSchema[T <: CsvRecord : ClassTag] = {
val clazz = implicitly[ClassTag[T]].runtimeClass
- val fields = clazz match {
- case _ if classOf[CsvRecordWithGroup] == clazz => CsvRecordWithGroup.fields
- case _ if classOf[CsvRecordNoGroup] == clazz => CsvRecordNoGroup.fields
+ val fields = {
Review comment:
Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #9643: MINOR: Upgrade to Scala 2.13.4 and 2.12.13
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9643:
URL: https://github.com/apache/kafka/pull/9643#discussion_r529325622
##########
File path: core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala
##########
@@ -151,21 +149,22 @@ object ConsumerGroupCommand extends Logging {
private[admin] case class CsvUtils() {
val mapper = new CsvMapper with ScalaObjectMapper
mapper.registerModule(DefaultScalaModule)
- def readerFor[T <: CsvRecord: ClassTag] = {
+ def readerFor[T <: CsvRecord : ClassTag] = {
val schema = getSchema[T]
val clazz = implicitly[ClassTag[T]].runtimeClass
mapper.readerFor(clazz).`with`(schema)
}
- def writerFor[T <: CsvRecord: ClassTag] = {
+ def writerFor[T <: CsvRecord : ClassTag] = {
val schema = getSchema[T]
val clazz = implicitly[ClassTag[T]].runtimeClass
mapper.writerFor(clazz).`with`(schema)
}
- private def getSchema[T <: CsvRecord: ClassTag] = {
+ private def getSchema[T <: CsvRecord : ClassTag] = {
val clazz = implicitly[ClassTag[T]].runtimeClass
- val fields = clazz match {
- case _ if classOf[CsvRecordWithGroup] == clazz => CsvRecordWithGroup.fields
- case _ if classOf[CsvRecordNoGroup] == clazz => CsvRecordNoGroup.fields
+ val fields = {
Review comment:
useless brackets
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma merged pull request #9643: MINOR: Upgrade to Scala 2.13.4
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #9643:
URL: https://github.com/apache/kafka/pull/9643
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org