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

Enable the development of custom consumers #689

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Enable the development of custom consumers #689

merged 1 commit into from
Aug 14, 2024

Conversation

manifest
Copy link
Contributor

@manifest manifest commented Jun 2, 2024 •
edited
Loading

This PR for the issue.

@manifest manifest mentioned this pull request Jun 2, 2024
Closed
@manifest manifest mentioned this pull request Jul 9, 2024
Open
@fede1024
Copy link
Owner

fede1024 commented Aug 5, 2024

Thank you for the contribution. Can you please add a test to verify that the callback is getting called. Thanks

@manifest
Copy link
Contributor Author

manifest commented Aug 5, 2024

Sure, I will be able to look into it soon.

Do you have any suggestion on how I can trigger native_message_queue_nonempty_cb callback in the test?

@manifest
Copy link
Contributor Author

manifest commented Aug 9, 2024

I've added the test.

@fede1024
Copy link
Owner

Thank you for adding the test. Can we produce a real message instead of calling rd_kafka_queue_yield? Tests are supposed to run with a working Kafka deployment alongside it (via docker compose), so you should be able to produce and consume from real topics.

@manifest
Enable the development of custom consumers
5b91bb4 
It is currently impossible to develop a custom consumer based on `BaseConsumer`
because its `queue` property, which is necessary to receive notifications about
new incoming messages, is private.

This defines `set_nonempty_callback` method on `BaseConsumer` similarly to how
it has already been done for `PartitionQueue`. That will allow setting
`rdkafka_sys::rd_kafka_queue_cb_event_enable` callback from within a custom
consumer implementation.
@manifest
Copy link
Contributor Author

I've moved the test to the tests/test_low_consumers module and replicated scenarios from test_produce_consume_message_queue_nonempty_callback.

Statistics is turned off to prevent interference with the wakeups counter.

@manifest
Copy link
Contributor Author

The failure of the test on the 7.5.1 + 3.5 option doesn't indicate an issue with the changes in the current PR, as the test also fails on the existing test_produce_consume_message_queue_nonempty_callback test.

@fede1024 fede1024 merged commit d739960 into fede1024:master Aug 14, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@manifest @fede1024