-
Notifications
You must be signed in to change notification settings - Fork 46
feat: upgrade to Kafka 4.0.0 #562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Since kafka is now compiled for scala 3 (right?) do we still need |
Unless I missed something, it's not published for Scala 3. Only 2.13. |
|
Okay, sorry, my bad. |
|
For some reason, unit tests relying on "arbitrary available ports" ( EDIT: also fails on CI. Will fix tomorrow. EDIT 2: error is likely due to the fact that the controller is started on a random port, but the broker is configured to reach the controller on port 0 instead of the actual port. This is a bit more tricky than expected because the random port is chosen by the controller when it starts. I may have to change the config of the broker after the controller has started. |
1e713a4 to
23b41b5
Compare
| with: | ||
| distribution: 'temurin' | ||
| java-version: 8 | ||
| java-version: 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kafka Connect & Server are not available for Java < 17 anymore: https://kafka.apache.org/40/documentation/compatibility.html
| lazy val prodDeps: Seq[ModuleID] = Seq( | ||
| "org.apache.kafka" %% "kafka" % Versions.Kafka cross CrossVersion.for3Use2_13 | ||
| ) | ||
| lazy val testDeps: Seq[ModuleID] = Seq( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ️ The library is not pulled transitively anymore but we do use it in our tests.
embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/kafkaOps.scala
Outdated
Show resolved
Hide resolved
0b751c9 to
ee9ab47
Compare
|
I got unit tests to pass. Next step: run some integration tests on projects of my company + on OSS projects like zio-kafka. |
I can run the test suite of zio-kafka. Is there a snapshot version somewhere? |
Not yet, I was planning to build one locally and run the tests locally on my machine. |
|
@erikvanoosten just to keep you informed that I started trying on the zio-kafka codebase. There are a couple of errors. Some are due to changes in 4.x that need zio-kafka to adapt as well, some may be due to issues with embedded-kafka. I haven't had time to look at it closely for now. Feel free to checkout this PR and build it locally if you want to try on your end for zio-kafka. I haven't tried yet on other projects. |
|
@gaeljw Did you already try it on our kafka 4 branch? |
I had not see there was already a branch. Oops! I'll start from it then :) |
| * @return | ||
| * the list of topic names | ||
| */ | ||
| def listTopics()(implicit config: C): Try[Set[String]] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added listTopics() and describeTopics() methods for our own tests. Previously we were asserting topics creation/properties by using a ZK client and reading from ZK but now we have to do differently. I chose to reuse the Kafka AdminClient.
embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/kafkaOps.scala
Show resolved
Hide resolved
|
I've tested on a few of my companies projects (simple usages), no issue found. Even though the upgrade PR in zio-kafka is failing, I believe these errors are not directly related to embedded-kafka and I would suggest to release a 4.0.0 of embedded-kafka. This will allow the community to use it and if they're any issue, we'll obviously fix them by releasing new 4.0.0.x versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request upgrades the project to Kafka 4.0.0, updating documentation and build configurations to reflect the new requirements and changes in functionality. Key changes include:
- Updated README.md with a version compatibility matrix, upgrade notes, and replacement of Zookeeper settings with controller settings.
- Adjusted GitHub workflow configurations to use Java 17 instead of Java 8.
Reviewed Changes
Copilot reviewed 6 out of 22 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| README.md | Updated documentation for upgraded Kafka version and configuration changes. |
| .github/workflows/test.yml | Changed Java version from 8 to 17 for tests. |
| .github/workflows/release.yml | Changed Java version from 8 to 17 for release workflows. |
| .github/workflows/coverage-report.yml | Changed Java version from 8 to 17 for coverage reporting. |
Files not reviewed (16)
- embedded-kafka/src/main/scala/io/github/embeddedkafka/EmbeddedKafka.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/EmbeddedKafkaConfig.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/EmbeddedServer.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/AdminOps.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/ConsumerOps.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/embeddedKafkaOps.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/kafkaOps.scala: Language not supported
- embedded-kafka/src/main/scala/io/github/embeddedkafka/ops/zooKeeperOps.scala: Language not supported
- embedded-kafka/src/test/scala/io/github/embeddedkafka/EmbeddedKafkaMethodsSpec.scala: Language not supported
- embedded-kafka/src/test/scala/io/github/embeddedkafka/EmbeddedKafkaObjectSpec.scala: Language not supported
- embedded-kafka/src/test/scala/io/github/embeddedkafka/EmbeddedKafkaWithRunningKafkaOnFoundPortSpec.scala: Language not supported
- embedded-kafka/src/test/scala/io/github/embeddedkafka/EmbeddedKafkaWithRunningKafkaSpec.scala: Language not supported
- kafka-connect/src/main/scala/io/github/embeddedkafka/connect/EmbeddedKafkaConnect.scala: Language not supported
- kafka-connect/src/test/scala/io/github/embeddedkafka/connect/ExampleKafkaConnectSpec.scala: Language not supported
- kafka-streams/src/main/scala/io/github/embeddedkafka/streams/EmbeddedKafkaStreams.scala: Language not supported
- kafka-streams/src/test/scala/io/github/embeddedkafka/streams/ExampleKafkaStreamsSpec.scala: Language not supported
Co-authored-by: Copilot <[email protected]>
|
@francescopellegrini , @guizmaii or @NeQuissimus: would one of you have time to give a look to the PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thanks @erikvanoosten (and welcome on board!) . I'll ship the release in the afternoon :) |
Resolves #561, supersedes #560
Tasks