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

Adds Kafka based StateRepository implementation #307

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

florian-stefan
Copy link

This PR adds a KafkaStateRepository. A detailed explanation of the functionality can be found in the Javadoc of the class KafkaStateRepository. It uses the standard Apache Kafka client library for connecting to Kafka as well as Gson serializing/deserializing feature states. Furthermore, the KafkaStateRepositoryTest uses the Kafka-JUnit4 test library from Salesforce for starting an in-memory Kafka cluster.

The file template.mf needs special attention because I was not really sure what to do here ... 😄.

Resolves: #304

Copy link

@swen-fuhrmann swen-fuhrmann left a comment

Choose a reason for hiding this comment

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

I like the good documentation.
The logging is quite verbose on info level. I wonder if it make sense to log more on debug level.

@florian-stefan
Copy link
Author

@smoczarski Thank you so much for your review! This is very helpful. I will add the suggested improvements as soon as possible!

@florian-stefan
Copy link
Author

@smoczarski I agree that the logging was quite verbose: I decreased the log level of some logs to 'debug'.

@chkal
Copy link
Member

chkal commented Nov 17, 2018

Hi! Thanks a lot for creating this PR.

I just stepped down from my role as the Togglz maintainer and from being an active committer.

But I hope that some other @togglz/committer can help with the review.

@chkal
Copy link
Member

chkal commented Dec 1, 2018

I just mentioned your pull request in my email discussing the future of Togglz. I hope this will help to get more reviews. I'm really sorry for the delay, but you created this pull request in a very awkward period of the project. :-)

I don't know much about Kafka, so I'm not sure if my review is very helpful. However, there is one thing I would like to mention. Togglz is also creating a "distribution ZIP" file for people not using Maven. Could you perhaps include the new module in this distribution? Just have a look at this directory. It should be easy to understand if you look at the pom and the assembly descriptor.

Thanks a lot!

@chkal chkal mentioned this pull request Dec 1, 2018
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.

None yet

3 participants