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

Refactor messaging. #57

Closed
wants to merge 2 commits into from
Closed

Refactor messaging. #57

wants to merge 2 commits into from

Conversation

mwarzynski
Copy link
Contributor

@mwarzynski mwarzynski commented Nov 30, 2018

At the beginning I wanted to fix logging an error coming from processing a message (retry). And a couple of minutes later I was in the process of refactoring the whole package...

  • I fixed the race condition related to setting readFunc in the Reader,
  • I unified Reader constructors,
  • I removed dependency to config provider (params might be provided by ReaderOptions),
  • I added handling context in messages reader (we may stop reading messages when server stops properly)
  • I added functionality of stopping reading from kafka reader (we may stop one reader and then start reading with another function),
  • I added better logging (for example, all messages contain topic and have the same format)
  • I added Brokers struct which allows to remove the duplications of code between our internal projects. We may just initialize this simple struct in the main function and use it to construct Readers/Writers without passing brokers as an argument.
  • I removed blank lines from tests.

The changes are breaking backwards compatibility. Nevertheless, it should be easy to fix after the dependency upgrade.

@mwarzynski mwarzynski closed this Dec 8, 2018
@mwarzynski mwarzynski deleted the refactor-messaging branch December 8, 2018 16:50
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.

1 participant