Skip to content
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

SourceWithOffsetContext #62

Merged
merged 14 commits into from
Oct 1, 2019

Conversation

IgorFedchenko
Copy link
Contributor

As a part of work over issue #36 , SourceWithOffsetContext consumer source will be implemented and tested in this PR.

This souse is basically provides another way of passing consumed messages and committable offset to downstream. The main difference (as far as I understand) is that while CommittableSource passes CommittableMessage to the downstream, forcing consequent flows to handle both message and it's committable offset, this SourceWithOffsetContext allows to put committable offset to the source's context.

Also, some helpers like Committer static class with committing flows are implemented in this PR, because they are used to make testing simpler/cleaner.

There is one more test for this source type, but it will require one more producer flow implementation, so it will be added in one of the next PRs.

@Aaronontheweb
Copy link
Member

@IgorFedchenko have some merge conflicts here

@IgorFedchenko
Copy link
Contributor Author

Yes, each of my PRs adds new extension method to KafkaConsumer.cs file, so seems like we will have conflicts on each merge...

@Aaronontheweb
Copy link
Member

@IgorFedchenko ah, ok

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, but have some questions

@IgorFedchenko
Copy link
Contributor Author

After updating committer default settings in reference.conf file one of the tests failed because I was expecting 10 messages on the downstream, but there are less of them due to batchSize > 1 (so now expecting elemsCount/batchSize committable batches).

@Aaronontheweb Aaronontheweb merged commit 669d1a1 into akkadotnet:dev Oct 1, 2019
@IgorFedchenko IgorFedchenko deleted the source_with_offset branch October 2, 2019 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants