Custom serialization#61
Open
danieroux wants to merge 7 commits intoonyx-platform:masterfrom
Open
Conversation
- This is a refactoring, does not change behaviour or expose new behaviour - Still uses byte-array-serializer as the default
lbradstreet
requested changes
Nov 20, 2018
Member
lbradstreet
left a comment
lbradstreet
left a comment
There was a problem hiding this comment.
Thanks! Looks good to merge once my minor comments are addressed.
| :default 2000 | ||
| :optional? true} | ||
|
|
||
| :kafka/deserializer |
Member
There was a problem hiding this comment.
:kafka/deserializer -> :kafka/value-deserializer for consistency with kafka.
Author
There was a problem hiding this comment.
This one is a tricky one for me.
I was keeping it consistent with :kafka/deserializer-fn - if you change the :kafka/deserializer you have to consider how it affects your :kafka/deserializer-fnchoice.
I changed the documentation to say something about it.
Do you still want me to change it to be consistent with kafka?
lbradstreet
reviewed
Nov 20, 2018
…ray is expected - Also assert in the test that the key/value in those functions are what we expect
- Preserve take-now signature before custom serializers were added - Allow end-offsets and beginning-end-offsets-clj to be used with custom serializers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This implements #60
I made the configs first-class keys, and not using kafka/consumer-opts. Consumer-opts feels more like "whatever this plugin did not consider".