[#555] Add option to commit offsets #556
Conversation
|
|
smcvb
requested review from
a team,
CodeDrivenMitch,
smcvb and
MateuszNaKodach
and removed request for
a team
smcvb
changed the title
There was a problem hiding this comment.
Conceptually I think there's nothing wrong with the changes. So, thanks for the effort there, @bradskuse! However, I do have some minor concerns we need to address. So, if you could take a look at those once you have found the time for that, that would be great.
| AUTO, | ||
| COMMIT_ASYNC, | ||
| COMMIT_SYNC |
There was a problem hiding this comment.
Nit: Each enumeration option should have some JavaDoc! Would you mind adding that, @bradskuse?
| package org.axonframework.extensions.kafka.eventhandling.consumer; | ||
|
|
||
| /** | ||
| * Enum to define how the consumer will handle committing offsets. |
There was a problem hiding this comment.
Feel free to add an @author tag with your name, and an @since tag referring to 4.11.1 to the class-level JavaDoc! :-)
| @@ -0,0 +1,10 @@ | |||
| package org.axonframework.extensions.kafka.eventhandling.consumer; | |||
There was a problem hiding this comment.
New files should have a copyright notice to state all is Apache 2 licensed. You can find the copyright notice template here. Or, you can copy it from another file in this repository and adjust the date accordingly.
| * @return the current Builder instance, for fluent interfacing | ||
| */ | ||
| public AsyncFetcher.Builder<K, V, E> offsetCommitType(OffsetCommitType offsetCommitType) { | ||
| this.offsetCommitType = offsetCommitType; |
There was a problem hiding this comment.
Nit: Please verify that the given offsetCommitType is not null. You can use the assertNonNull method from the Builder#executorService method, for that, for example.
| * Set the {@code offsetCommitType}, options are: | ||
| * {@link OffsetCommitType#AUTO} let the Kafka consumer commit offsets automatically in background | ||
| * {@link OffsetCommitType#COMMIT_SYNC} let the Kafka consumer commit offsets synchronously after processing | ||
| * {@link OffsetCommitType#COMMIT_ASYNC} let the Kafka consumer commit offsets asynchronously after processing | ||
| * Defaults to {@code OffsetCommitType#AUTO} |
There was a problem hiding this comment.
Minor restructuring suggestion:
| * Set the {@code offsetCommitType}, options are: | |
| * {@link OffsetCommitType#AUTO} let the Kafka consumer commit offsets automatically in background | |
| * {@link OffsetCommitType#COMMIT_SYNC} let the Kafka consumer commit offsets synchronously after processing | |
| * {@link OffsetCommitType#COMMIT_ASYNC} let the Kafka consumer commit offsets asynchronously after processing | |
| * Defaults to {@code OffsetCommitType#AUTO} | |
| * Sets the {@code offsetCommitType} defining how the {@link FetchEventsTask} will commit offsets during processing of events. | |
| * | |
| * Options are: | |
| * <ul> | |
| * <li>{@link OffsetCommitType#AUTO} - let the Kafka consumer commit offsets automatically in background.</li> | |
| * <li>{@link OffsetCommitType#COMMIT_SYNC} - let the Kafka consumer commit offsets synchronously after processing.</li> | |
| * <li>{@link OffsetCommitType#COMMIT_ASYNC} - let the Kafka consumer commit offsets asynchronously after processing.</li> | |
| * </ul> | |
| * | |
| * Defaults to {@code OffsetCommitType#AUTO}, meaning the offset commit task happens in the background. |
There was a problem hiding this comment.
Changes to this file should update the copyright notice to this year. More specifically, that means line 2 of this file should state the following:
Copyright (c) 2010-2025. Axon FrameworkThere was a problem hiding this comment.
Changes to this file should update the copyright notice to this year. More specifically, that means line 2 of this file should state the following:
Copyright (c) 2010-2025. Axon FrameworkThere was a problem hiding this comment.
Changes to this file should update the copyright notice to this year. More specifically, that means line 2 of this file should state the following:
Copyright (c) 2010-2025. Axon Framework
|
Hey @smcvb Thank you for reviewing this PR. I have just pushed up all the changes requested 👍 |
Resolves #555
This adds an option so that offsets can be committed after processing of events, instead of using enable auto commit.
Edit: Added ability to use auto, async or sync