-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Increase kafka client test coverage #8237
Increase kafka client test coverage #8237
Conversation
Forgot to disable the test if docker is not enabled. Will fix that. |
545c6bf
to
3c88d1e
Compare
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, a few minor comments inline.
integration-tests/kafka/src/main/java/io/quarkus/it/kafka/avro/AvroEndpoint.java
Show resolved
Hide resolved
integration-tests/kafka/src/main/java/io/quarkus/it/kafka/sasl/SaslKafkaEndpoint.java
Show resolved
Hide resolved
integration-tests/kafka/src/test/java/io/quarkus/it/kafka/SaslKafkaConsumerTest.java
Outdated
Show resolved
Hide resolved
@cescoffier can you check Gunnar's comments? |
@gunnarmorling I need to look at the Apicurio version of the Avro serde. So far the need was the "original" Avro. I can see an opportunity here if it works flawlessly in native. |
dca5dfd
to
98762cf
Compare
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 really don't like the fact that we have versions in the pom files. They should either be in the BOM if it makes sense or in the build-parent.
As these artifacts are not available from Maven Central it's tricky to add them to the dependency management from the parent. That's why I put the version there on purpose. Someone not aware of this would add the dependency that won't get resolved. |
98762cf
to
937fa10
Compare
I rebased and force pushed as it was an old PR. I'm a bit worried about the external repository. We'll see if it causes problems and reevaluate. |
Fixes #8025
add a few tests related to the Kafka client.
Unfortunately, Avro does not work in native mode (fields written using unsafe).
I will create an issue to track this.