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.

Sign up for GitHub

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

Jump to bottom

Fix encoder #93

Merged
merged 3 commits into from
Jul 11, 2021
Merged

Fix encoder #93

merged 3 commits into from
Jul 11, 2021

Conversation

zmstone
Copy link
Contributor

@zmstone zmstone commented Jul 11, 2021 •
edited
Loading

fixes #78

zmstone added 3 commits July 11, 2021 16:15
@zmstone
fix(test): ensure connect partition leader for produce tests
cc1bb37
@zmstone
fix(encoder): empty 'bytes' is not 'null'
ff394b8 
Kafka APIs seem to treat 'empty' and 'null' the same way,
however, SASL auth API has more strict check.
@zmstone
fix(encoder): encode 'null' for non-nullable string as 'empty'
162b851 
Otherwise the produce API for non-transactional request uses
"undefined" as transaction ID.
@zmstone zmstone requested review from qzhuyan and k32 July 11, 2021 14:24
qzhuyan
qzhuyan approved these changes Jul 11, 2021
View reviewed changes
src/kpro_lib.erl Show resolved Hide resolved
@zmstone zmstone merged commit d0d3b78 into kafka4beam:master Jul 11, 2021
@zmstone zmstone deleted the fix-encoder branch July 11, 2021 15:10
@vikas15bhardwaj
Copy link

@zmstone Thanks for working on this issue. I tested with latest :brod and it pulled 4.0.1 kafka_protocol version. I noticed the change didn't pull as expected. I then checked release tagged 4.0.1 code and it seems tagged release is still having old code. I then checked master code branch which has the correct code. Can you check this.

4.0.1 code

kafka_protocol/src/kpro_lib.erl

Line 147 in 94643e1

encode(bytes, B) when is_binary(B) orelse is_list(B) ->

@vikas15bhardwaj vikas15bhardwaj mentioned this pull request Jul 23, 2021
Merged
@zmstone
Copy link
Contributor Author

zmstone commented Jul 24, 2021

Hi @vikas15bhardwaj 4.0.1 is old.
This fix is merged to master branch, but not released yet.

My plan was to include it in 4.1.0
#94

@zmstone
Copy link
Contributor Author

zmstone commented Jul 24, 2021

Just release 4.0.2. also published to hex.

@vikas15bhardwaj
Copy link

Thanks @zmstone for publishing 4.0.2 release. My next problem is updating brod to use this release.

@vikas15bhardwaj
Copy link

vikas15bhardwaj commented Jul 24, 2021 •
edited
Loading

@zmstone I was looking at updating brod to use kafka_protocol 4.0.2, I hope it's just a change in rebar.config file. Wondering the setup needed to compile/test. Seems no README on that. Can you help with it

@zmstone
Copy link
Contributor Author

zmstone commented Jul 24, 2021

Hi @vikas15bhardwaj

You are right, just change dependency version should be enough.
You can try to pin kafka_protocol version in your project for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qzhuyan qzhuyan qzhuyan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Encoding 0 bytes behavior
3 participants
@zmstone @vikas15bhardwaj @qzhuyan