confluent-kafka-python
confluent-kafka-python copied to clipboard
confluentinc
bool(Producer(...)) returns False
Description
It's not necessarily a bug, the Producer class has a False-y truth value when there's no enqueued items. This is caused by the combination of the default nb_bool implementation (https://github.com/python/cpython/blob/main/Objects/typeobject.c#L7915), which checks for the existence of __bool__ and if it doesn't exist, it returns the result of __len__. This works well with normal containers, like lists/dictionaries, but feels off for a kafka producer. Moreover it behaves strangely for idiomatic python like:
def default_producer():
...
def send_messages(producer, ...):
# producer will evaluate to False if there's no queued messages
producer = producer or default_producer()
It's not obvious if this is the intended behaviour, and "fixing" it would be a breaking change, but I thought it's quite weird. Perhaps just updating the docs might be enough.
How to reproduce
>>> confluent_kafka.version()
('1.8.2', 17302016)
>>> confluent_kafka.libversion()
('1.8.2', 17302271)
>>> from confluent_kafka import Producer, Consumer
>>> bool(Producer({}))
%5|1663241720.611|CONFWARN|rdkafka#producer-2| [thrd:app]: No `bootstrap.servers` configured: client will not be able to connect to Kafka cluster
False
>>> bool(Consumer({'group.id': 'asd'}))
True
Checklist
Please provide the following information:
- [x] confluent-kafka-python and librdkafka version (
confluent_kafka.version()andconfluent_kafka.libversion()): - [ ] Apache Kafka broker version:
- [x] Client configuration:
{...} - [ ] Operating system:
- [ ] Provide client logs (with
'debug': '..'as necessary) - [ ] Provide broker log excerpts
- [ ] Critical issue
nice observation!
My view is that using __len__ the way Producer does probably wasn't a good idea. I don't think we should ever change this though because it is breaking, and would impact more people than is justified by the change. Changing the defn of nb_bool would also be breaking, but seems unlikely to affect anyone (?) famous last words though, we've done breaking changes under similar justification before and mucked people up... overall i'm in favor of doing it. I think small API niceties like this add up and amount to happy users.
what do you think @edenhill ?
