Skip to content

Support for Message Headers #114

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

Closed
rriggs opened this issue Sep 20, 2018 · 8 comments
Closed

Support for Message Headers #114

rriggs opened this issue Sep 20, 2018 · 8 comments

Comments

@rriggs
Copy link

rriggs commented Sep 20, 2018

librdkafka added support to the C API for message headers in release 0.11.4.

Any chance of adding message header support to MessageBuilder in the near future?

@mfontanini
Copy link
Owner

I wanted to add support for headers but I never really sat down and implemented that. I'm currently working on other stuff so I won't be able to write this in a while, but we should definitely add support at some point.

@accelerated
Copy link
Contributor

I could take a look. Actually in my cppkafka wrapper we added support for headers inside the packet itself as it was a requirement. The header has a small shim with a version number a a header length which points to the beginning of the serialized message. Anything between the shim and the message is the header so it can be extracted separately, so I will see if it make sense to modify our code and this one. I am also trying to see if we can open source it so it could be maintained more easily going forward.

@accelerated
Copy link
Contributor

Ok so I'll work on this, starting next week probably. I think it makes sense to fully support it. I'll issue a design review before coding it so that @mfontanini can approve the overall concept.

@mfontanini
Copy link
Owner

Yeah, this definitely should be supported if someone can put the manpower into building it. I had some thoughts about it when I was considering implementing it but they all faded away by now. Thanks for taking the initiative on this one!

@accelerated
Copy link
Contributor

Sure!

@accelerated
Copy link
Contributor

Guess we can close this issue now?

@mfontanini
Copy link
Owner

Yep, definitely. This is closed by #115. Btw if you write "fixes #114" in the PR comments I think after the merge related issue gets closed automatically.

@accelerated
Copy link
Contributor

Cool, didn't know about this. Will use it next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants