Consumer commit() None parameters

[beszabo1]

Description

(Changing a discussion to an issue, since it is fairly straightforward)

From python, calling a Consumer's commit function with parameters set to None (or other python False values) will resolve as boolean true in Consumer.c's checks, so instead of disregarding them, the function returns different errors.

How to reproduce

The issue can be seen when calling one of these:

consumer.commit(message=None)
consumer.commit(offsets=None)
consumer.commit(message=None, offsets=None)

But as a note:

kwargs = {'async': False }
consumer.commit(**kwargs)

and

consumer.commit(asynchronous=False)

work as expected.

Additionally

I suspect this is the line where it happens, but am not familiar: https://github.com/confluentinc/confluent-kafka-python/blob/master/src/confluent_kafka/src/Consumer.c#L468C12-L468C12

[joaoe]

I suspect this is the line where it happens, but am not familiar:

I think you are correct. This if check is incorrect if (msg && offsets) because msg and offsets are set to the Py_None value, which is not NULL and evaluates as true in those if checks in the C code.

[beszabo1]

Py_None value, which is not NULL and evaluates as true

Ah that explains it then, and async only works because

if (async_o)
		async = PyObject_IsTrue(async_o);

always evaluates as true first, but PyObject_IsTrue handles it afterwards correctly, so it could be used for message and offsets in the checks.

[joaoe]

so it could be used for message and offsets in the checks.

Not really. That function expects a MeSsage object or a list of partitions. So using that True check would break for stuff that is not None and not those types.

This should be enough and does not need any more changes to the rest of the code.

msg = msg == Py_None ? NULL : msg;
offsets = offsets == Py_None ? NULL : offsets;