-
Notifications
You must be signed in to change notification settings - Fork 0
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
Log confirm messages #44
Conversation
5e3c4c4
to
94335f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First commit looks good.
Added a few questions for the second commit
2b169c4
to
7fa66ce
Compare
Looks like you have a few CI errors, not sure if you have seen them or not: I'll wait with the next round of review until those errors have been fixed, sounds good @andrbo ? |
7fa66ce
to
c648c08
Compare
c648c08
to
dfdd078
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider one final time if you want to move the context and reader into the implementation function
dfdd078
to
d4ab454
Compare
As both verify reject/confirm is similar, a refactor has been made to allow for sharing of code used by both. - Setup of the kafka reader been moved to the cmd argument parsing, to simplify function calls down the line, and hopefully making it easier to mock in tests. - Changed the groupID in the kafka reader config to separate between verify confirm and verify reject - Now supplying context to the read functions, for a graceful shutdown when running i within a container.
Heavily inspired by the verify reject command this command now prints the messages from confirm-topic.
3c16a01
to
3a4a4a4
Compare
Aims to solve #3 , by logging messages from the
confirm-topic
.This pull request will add loggin of the
identifier
andpath
from the message.