-
Notifications
You must be signed in to change notification settings - Fork 421
fix(parser): make key attribute optional in Kafka model #6523
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
Conversation
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
Dear @leandrodamascena, |
…w Kafka record without key into json files and added tests
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.
Hey @Weugene thanks a lot for sending this PR to fix this issue! I see that SonarCloud is complaining about some assertions and I appreciate if you can take a look! If you can't find the issue, I can try to check it tomorrow.
btw, thanks for fixing it in event sources too,
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6523 +/- ##
========================================
Coverage 96.16% 96.16%
========================================
Files 243 243
Lines 11838 11842 +4
Branches 885 886 +1
========================================
+ Hits 11384 11388 +4
Misses 356 356
Partials 98 98 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…a record with explicit setting "key": null into kafkaEventMsk.json and kafkaEventSelfManaged.json.
|
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.
Super nice PR @Weugene! Thanks for addressing all the comments.
APPROVED!
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Issue number: #6508
Summary
Changes
According to Kafka documentation, key in Kafka messages is optional parameter:
User experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.