-
Notifications
You must be signed in to change notification settings - Fork 934
kafkatest, refactoring, and bug fixes #1
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
Changes from 8 commits
77305ff
5433bb8
4592b31
6d90fa7
e9c9570
b520c8d
b2568b4
0dad9ef
99b774b
e6d3877
6936d9f
ee588bf
f853b46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| __all__ = ['cimpl','kafkatest'] | ||
| from .cimpl import * |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,10 @@ static int Consumer_clear (Consumer *self) { | |
| Py_DECREF(self->on_revoke); | ||
| self->on_revoke = NULL; | ||
| } | ||
| if (self->on_commit) { | ||
| Py_DECREF(self->on_commit); | ||
| self->on_commit = NULL; | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
|
|
@@ -518,7 +522,7 @@ static PyObject *Consumer_new (PyTypeObject *type, PyObject *args, | |
|
|
||
| PyTypeObject ConsumerType = { | ||
| PyVarObject_HEAD_INIT(NULL, 0) | ||
| "confluent_kafka.Consumer", /*tp_name*/ | ||
| "cimpl.Consumer", /*tp_name*/ | ||
| sizeof(Consumer), /*tp_basicsize*/ | ||
| 0, /*tp_itemsize*/ | ||
| (destructor)Consumer_dealloc, /*tp_dealloc*/ | ||
|
|
@@ -543,6 +547,18 @@ PyTypeObject ConsumerType = { | |
| "\n" | ||
| " Create new Consumer instance using provided configuration dict.\n" | ||
| "\n" | ||
| " Special configuration properties:\n" | ||
| " ``on_commit``: Optional callback will be called when a commit " | ||
| "request has succeeded or failed.\n" | ||
| "\n" | ||
| "\n" | ||
| ".. py:function:: on_commit(consumer, err, partitions)\n" | ||
| "\n" | ||
| " :param Consumer consumer: Consumer instance.\n" | ||
| " :param KafkaError err: Commit error object, or None on success.\n" | ||
| " :param list(TopicPartition) partitions: List of partitions with " | ||
| "their committed offsets or per-partition errors.\n" | ||
| "\n" | ||
| "\n", /*tp_doc*/ | ||
| (traverseproc)Consumer_traverse, /* tp_traverse */ | ||
| (inquiry)Consumer_clear, /* tp_clear */ | ||
|
|
@@ -566,8 +582,8 @@ PyTypeObject ConsumerType = { | |
|
|
||
|
|
||
| static void Consumer_rebalance_cb (rd_kafka_t *rk, rd_kafka_resp_err_t err, | ||
| rd_kafka_topic_partition_list_t *c_parts, | ||
| void *opaque) { | ||
| rd_kafka_topic_partition_list_t *c_parts, | ||
| void *opaque) { | ||
| Consumer *self = opaque; | ||
|
|
||
| PyEval_RestoreThread(self->thread_state); | ||
|
|
@@ -590,6 +606,7 @@ static void Consumer_rebalance_cb (rd_kafka_t *rk, rd_kafka_resp_err_t err, | |
| cfl_PyErr_Format(RD_KAFKA_RESP_ERR__FAIL, | ||
| "Unable to build callback args"); | ||
| self->thread_state = PyEval_SaveThread(); | ||
| self->callback_crashed++; | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -622,6 +639,51 @@ static void Consumer_rebalance_cb (rd_kafka_t *rk, rd_kafka_resp_err_t err, | |
| } | ||
|
|
||
|
|
||
| static void Consumer_offset_commit_cb (rd_kafka_t *rk, rd_kafka_resp_err_t err, | ||
| rd_kafka_topic_partition_list_t *c_parts, | ||
| void *opaque) { | ||
| Consumer *self = opaque; | ||
| PyObject *parts, *k_err, *args, *result; | ||
|
|
||
| if (!self->on_commit) | ||
| return; | ||
|
|
||
| PyEval_RestoreThread(self->thread_state); | ||
|
|
||
| /* Insantiate error object */ | ||
| k_err = KafkaError_new_or_None(err, NULL); | ||
|
|
||
| /* Construct list of TopicPartition based on 'c_parts' */ | ||
| parts = c_parts_to_py(c_parts); | ||
|
|
||
| args = Py_BuildValue("(OOO)", self, k_err, parts); | ||
|
|
||
| Py_DECREF(k_err); | ||
| Py_DECREF(parts); | ||
|
|
||
| if (!args) { | ||
| cfl_PyErr_Format(RD_KAFKA_RESP_ERR__FAIL, | ||
| "Unable to build callback args"); | ||
| self->thread_state = PyEval_SaveThread(); | ||
| self->callback_crashed++; | ||
| return; | ||
| } | ||
|
|
||
| result = PyObject_CallObject(self->on_commit, args); | ||
|
|
||
| Py_DECREF(args); | ||
|
|
||
| if (result) | ||
| Py_DECREF(result); | ||
| else { | ||
| self->callback_crashed++; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we be doing anything else if the callback fails? I know we return
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning NULL from a C python method makes it automatically raise any exceptions generated during function execution, so we should be good here. |
||
| rd_kafka_yield(rk); | ||
| } | ||
|
|
||
| self->thread_state = PyEval_SaveThread(); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| static PyObject *Consumer_new (PyTypeObject *type, PyObject *args, | ||
| PyObject *kwargs) { | ||
|
|
@@ -640,6 +702,7 @@ static PyObject *Consumer_new (PyTypeObject *type, PyObject *args, | |
| } | ||
|
|
||
| rd_kafka_conf_set_rebalance_cb(conf, Consumer_rebalance_cb); | ||
| rd_kafka_conf_set_offset_commit_cb(conf, Consumer_offset_commit_cb); | ||
|
|
||
| self->rk = rd_kafka_new(RD_KAFKA_CONSUMER, conf, | ||
| errstr, sizeof(errstr)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember if passing the consumer instance as part of the callback came up before. I checked and we have it elsewhere. Is this valuable? Often in Python you just grab what you need via closure.