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

Added option to customize partition events by stage logic subclasses #52

Merged

Conversation

IgorFedchenko
Copy link
Contributor

This is a small PR, which adds a way to customize partition events handling by different stage logic classes.

Before this PR partitions events were handled by SingleSourceStageLogic without any option to override it. Besides, original kafka driver callbacks also provide consumer instance passed to callbacks, so actually handling could use some consumer's methods.

Now consumer is passed to this callbacks, and the callbacks itself may be customized by logic subclasses (looks like only for TransactionalSourceLogic so far).

@IgorFedchenko
Copy link
Contributor Author

During PR #48 review process, I thought that IPartitionEventHandler interface will be made public. But currently all references in original alpakka project, that use this interface, are private/internal. So actually making it public will not give library users any option of use this interface - so keeping it internal so far. We can switch it to public whenever there will be any option to use it from outside.

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 to me, but left 1 question

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.

LGTM

@Aaronontheweb
Copy link
Member

@IgorFedchenko FYI, on the previous commit had one test failure:

Akka.Streams.Kafka.Tests.Akka.Streams.Kafka.Tests.Integration.PlainSourceIntegrationTests.PlainSource_consumes_messages_from_KafkaProducer_with_subscribe_to_topic

You can see the logs and such if you click through here: https://github.com/akkadotnet/Akka.Streams.Kafka/runs/227053848

Might have a race condition of some kind with that test.

@Aaronontheweb Aaronontheweb merged commit 7de4f82 into akkadotnet:dev Sep 18, 2019
@IgorFedchenko IgorFedchenko deleted the partitions_handler_customization branch September 18, 2019 18:33
@IgorFedchenko
Copy link
Contributor Author

IgorFedchenko commented Sep 18, 2019

@Aaronontheweb Good point. Created issue for that - looks little bit wierd, so unfortunately can not fix this right now, "on the fly".

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