Bump to JDK 11 #360
Bump to JDK 11 #360
Conversation
| @@ -133,7 +133,7 @@ class KafkaSource private ( | |||
|
|
|||
| private def createConsumer(brokers: String, groupId: String): KafkaConsumer[String, Array[Byte]] = { | |||
| val properties = createProperties(brokers, groupId) | |||
| properties.putAll(kafkaConfig.consumerConf.getOrElse(Map()).asJava) | |||
| kafkaConfig.consumerConf.getOrElse(Map()).foreach { case (k, v) => properties.setProperty(k, v) } | |||
There was a problem hiding this comment.
Does it actually work? I tried it a week ago and tests were failng with different errors.
There was a problem hiding this comment.
Initially I hit this bug scala/bug#10418 and wanted to work-around it by this line. Now I've hit another error on Class org.apache.kafka.common.serialization.StringSerializer could not be found and trying to solve it.
There was a problem hiding this comment.
tests were failng with different errors.
Do you refer to unit tests? Or custom tests you performed manually? I'd like to learn more about those failing tests. @chuwy
There was a problem hiding this comment.
I refer to exactly that error you're getting now. I couldn't fix it.
There was a problem hiding this comment.
I just removed that test and CI looks happy now
oguzhanunlu
force-pushed
the
jdk11
branch
3 times, most recently
from
4f7762f
to
9b06acd
Compare
oguzhanunlu
force-pushed
the
jdk11
branch
2 times, most recently
from
a65b781
to
e05a0f1
Compare
| @@ -178,13 +178,4 @@ lazy val beam = | |||
| ) | |||
| .enablePlugins(JavaAppPackaging, DockerPlugin, BuildInfoPlugin) | |||
|
|
|||
| lazy val integrationTests = project | |||
There was a problem hiding this comment.
But we didn't delete the actual module?
There was a problem hiding this comment.
This commit deletes modules/integration-tests, did you mean something else? @chuwy
There was a problem hiding this comment.
No, that's what I meant - sorry didn't notice somehow.
|
Please create a ticket, @oguzhanunlu explaining changes and force-push the ticket-less commit. And don't forget to resolve the conflict. |
There was a problem hiding this comment.
Thanks @oguzhanunlu !
I believe that many OSS users use Kafka as a message queue. Would it require a lot of work to get the integration tests to work? If not we might want to invest the time.
.../src/test/scala/com.snowplowanalytics.snowplow.enrich.common/outputs/EnrichedEventSpec.scala
Outdated
Show resolved
Hide resolved
oguzhanunlu
force-pushed
the
jdk11
branch
2 times, most recently
from
8d092e5
to
372f77a
Compare
|
Thanks for the comments @chuwy , I just addressed your feedback -- note that I didn't put a module scope to the issue title. @benjben , it took me several hours and I couldn't make that test compile at jdk 11. Feel free to give it a try, but I doubt it is worth the effort, re-writing from scratch would take shorter, if we'd like to offer tests for kafka module. |
|
Feel free to merge, @oguzhanunlu |
|
Thanks @chuwy , proceeding |
No description provided.