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.

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

Allow sending & reading headers via Kafka Message object. #880

Closed
wants to merge 8 commits into from

Conversation

Steveb-p
Copy link
Contributor

Note that Topic::producev & KafkaMessage::headers exist since
phprdkafka >= 3.1.0.

Resolves #843

Due to the fact that php-enqueue/kafka might be used with different versions of phprdkafka, I opted to maintain serializing of headers into the message.

Steveb-p added 6 commits May 31, 2019 19:20
Note that `Topic::producev` & `KafkaMessage::headers` exist since
phprdkafka >= 3.1.0.
Note that `Topic::producev` & `KafkaMessage::headers` exist since
phprdkafka >= 3.1.0.
Note that `Topic::producev` & `KafkaMessage::headers` exist since
phprdkafka >= 3.1.0.
// Note: Topic::producev method exists in phprdkafka >= 3.1.0
// Headers in payload are maintained for backwards compatibility with apps that might run on lower phprdkafka version
if (method_exists($topic, 'producev')) {
$topic->producev($partition, 0 /* must be 0 */, $payload, $key, $message->getHeaders());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably also should contain a check against phprdkafka version & librdkafka 1.0.0, since there is a possible segfault here.

EDIT: Well, as a matter of fact, the PR that fixes the producev call for librdkafka 1.0.0 is not yet included in a release, so it will probably have to remain as a TODO once it does.

Related PR:
arnaud-lb/php-rdkafka#222

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added version check. I've tried it with local build from master and

var_dump(version_compare('3.1.0-dev', '3.1.0', '<=');
// false

which is the version that you will get if you build from master branch of phprdkafka.

I'm considering whether or not this check is worthwile, however otherwise it will result in hard to debug segfault.

@makasim what are you thought on this? Should it be dropped? Should it emit a warning (like here) or "drop down" to calling produce?

Steveb-p added a commit to Steveb-p/enqueue-dev that referenced this pull request Jun 7, 2019
A simpler part of php-enqueue#880, adds just the consumption part. This makes
headers accessible in the consumer in case another application makes
use of those, be it PHP or not.
@stale
Copy link

stale bot commented Jul 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 1, 2019
@stale stale bot closed this Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Kafka] Support headers
1 participant