-
Notifications
You must be signed in to change notification settings - Fork 106
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
Missing the ConsumerSeekAware interface #812
Conversation
92b9470
to
05ba997
Compare
05ba997
to
12debce
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.
Add tests for the code samples in test-suite
, test-suite-groovy
and `test-suite-kotlin
kafka/src/main/java/io/micronaut/configuration/kafka/ConsumerSeekAware.java
Show resolved
Hide resolved
kafka/src/main/java/io/micronaut/configuration/kafka/ConsumerSeekAware.java
Outdated
Show resolved
Hide resolved
kafka/src/main/java/io/micronaut/configuration/kafka/seek/DefaultKafkaSeekOperations.java
Outdated
Show resolved
Hide resolved
kafka/src/main/java/io/micronaut/configuration/kafka/seek/DefaultKafkaSeeker.java
Outdated
Show resolved
Hide resolved
kafka/src/main/java/io/micronaut/configuration/kafka/seek/KafkaSeekOperation.java
Outdated
Show resolved
Hide resolved
test-suite-kotlin/src/test/kotlin/io/micronaut/kafka/docs/seek/ProductListener.kt
Outdated
Show resolved
Hide resolved
test-suite-kotlin/src/test/kotlin/io/micronaut/kafka/docs/rebalance/ProductListener.kt
Outdated
Show resolved
Hide resolved
test-suite-groovy/src/test/groovy/io/micronaut/kafka/docs/seek/ops/ProductListener.groovy
Show resolved
Hide resolved
test-suite-groovy/src/test/groovy/io/micronaut/kafka/docs/rebalance/ProductListener.groovy
Outdated
Show resolved
Hide resolved
test-suite-groovy/src/test/groovy/io/micronaut/kafka/docs/seek/ProductListener.groovy
Outdated
Show resolved
Hide resolved
…the-consumerseekaware-interface
@guillermocalvo I merged master into the branch but the test fails for me. can you investigate? Sorry 😬 |
You removed |
This reverts commit 9cbba67.
ping @timyates 🖕. if possible it would be good to use the same Kafka instance for them. |
I removed it because MY_KAFKA property no longer exists since #824. |
@sdelamo Those classes were there to illustrate section Running Kafka while testing and developing of the guide. |
As @guillermocalvo notes But see #826 as an example I am updating other sections in the guide to do the same (i.e. multi-lang snippets with test-resources tests). |
@sdelamo I updated the tests to use test-resources. BTW SonarCloud is in the wrong again. |
test-suite-groovy/src/test/groovy/io/micronaut/kafka/docs/seek/aware/ProductListener.groovy
Outdated
Show resolved
Hide resolved
...roovy/src/test/groovy/io/micronaut/kafka/docs/seek/aware/ProductListenerConfiguration.groovy
Outdated
Show resolved
Hide resolved
...-groovy/src/test/groovy/io/micronaut/kafka/docs/seek/ops/ProductListenerConfiguration.groovy
Outdated
Show resolved
Hide resolved
test-suite-groovy/src/test/groovy/io/micronaut/kafka/docs/seek/rebalance/ProductListener.groovy
Show resolved
Hide resolved
...y/src/test/groovy/io/micronaut/kafka/docs/seek/rebalance/ProductListenerConfiguration.groovy
Outdated
Show resolved
Hide resolved
test-suite/src/test/java/io/micronaut/kafka/docs/seek/ops/ProductListener.java
Outdated
Show resolved
Hide resolved
test-suite/src/test/java/io/micronaut/kafka/docs/seek/ops/ProductListenerConfiguration.java
Outdated
Show resolved
Hide resolved
test-suite/src/test/java/io/micronaut/kafka/docs/seek/rebalance/ProductListener.java
Outdated
Show resolved
Hide resolved
test-suite/src/test/java/io/micronaut/kafka/docs/seek/rebalance/ProductListener.java
Show resolved
Hide resolved
...suite/src/test/java/io/micronaut/kafka/docs/seek/rebalance/ProductListenerConfiguration.java
Outdated
Show resolved
Hide resolved
ping @dstepanov |
kafka/src/main/java/io/micronaut/configuration/kafka/seek/DefaultKafkaSeeker.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
@NonNull | ||
private Optional<Boolean> performForZeroOffset(@NonNull KafkaSeekOperation operation, |
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 would use just nullable boolean
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.
Ok so I did what you requested but then Sonar was complaining about "null should not be returned from a Boolean method" and I had to revert the changes.
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.
Ok
Kudos, SonarCloud Quality Gate passed! |
Added two new mechanisms to allow for immediate/deferred Kafka seek operations:
ConsumerSeekAware
allows consumers to react when the set of partitions assigned to the consumer changes.KafkaSeekOperations
can be injected into consumer methods so that seek operations can be performed when/if the method completes successfully.