list_topics() crash

Description

Created a consumer, called list_topics(), immediate crash.

How to reproduce

There's a simple bug in the code, but it's only triggered by a somewhat bad installation environment. Let me explain: in my setup, I can import confluent_kafka, but if I do this:

import confluent_kafka.admin
Traceback (most recent call last):
File "", line 1, in
File "/usr/anim/modsquad/oses/rhel7/src/confluent-kafka-python-v1.0.0-PRE1/build/lib.linux-x86_64-2.7/confluent_kafka/admin/init.py", line 20, in
import concurrent.futures
ImportError: No module named concurrent.futures
import concurrent
Traceback (most recent call last):
File "", line 1, in
ImportError: No module named concurrent

OK, that's on me (though I don't see anywhere that I should have the concurrent module). But given that I can't import confluent_kafka.admin (though I can import confluent_kafka), if you look in the code for list_topics(), you'll see that eventually this call is made:

cfl_PyObject_lookup("confluent_kafka.admin",
"ClusterMetadata");

and in the code for cfl_PyObject_lookup you call
PyObject *module = PyImport_ImportModule(modulename);

but don't ever check that module is non-null. so basically, a bad setup leads to an inscrutable crash. Not high priority, but thought I'd point it out. maybe raise an exception if the PyImport_ImportModule fails and tell me the module couldn't be loaded? would have saved an hours worth of digging.

That way, you don't have to tell me to install the concurrent module: the stack trace will provide me with a clue.

FYI, I only figured this out from looking at the code -- i didn't know to try importing the admin module till i saw it in the code and backtracked it all.

deb@pixar.com
David Baraff

Overall, love this code. thanks for it!

Checklist

Please provide the following information:

• confluent-kafka-python and librdkafka version (confluent_kafka.version() and confluent_kafka.libversion()):
• Apache Kafka broker version:
• Client configuration: {...}
• Operating system:
• Provide client logs (with 'debug': '..' as necessary)
• Provide broker log excerpts
• Critical issue

Once I added not only concurrent.futures but also enum, works like a charm. and fast fast fast!

thank you so much for adding list_topics().

[rnpridgeon]

Hey David,

I'm not sure we actually have a bug here as ImportError actually is a valid built-in exception and it should be raised when certain dependencies are not satisfied.

Raised when an import statement fails to find the module definition or when a from ... import fails to find a name that is to be imported.

https://docs.python.org/2/library/exceptions.html#exceptions.ImportError

I'm not sure how exactly you installed the rc release but might I suggest using pip next time. This will actually call out the missing dependencies before installing the package.

pip install --index-url https://test.pypi.org/simple/  confluent-kafka==1.0.0rc1
Looking in indexes: https://test.pypi.org/simple/
Collecting confluent-kafka==1.0.0rc1
  Using cached https://test-files.pythonhosted.org/packages/15/10/144f62e629d644c7bda80113f1c63d33ef343b59ebf4c3b746ee33460288/confluent_kafka-1.0.0rc1-cp27-cp27m-macosx_10_6_intel.whl
Collecting enum34 (from confluent-kafka==1.0.0rc1)
  Could not find a version that satisfies the requirement enum34 (from confluent-kafka==1.0.0rc1) (from versions: )
No matching distribution found for enum34 (from confluent-kafka==1.0.0rc1)

If you wish to install the dev packages on test.pypi and their dependencies you can use --extra-index-url which will search both test.pyi and pypi the package and its dependencies.

pip install --extra-index-url https://test.pypi.org/simple/  confluent-kafka==1.0.0rc1

To be clear, you are calling import in C code. You get back a null pointer. You dereference it in C. You crash with a segmentation violation which gives very littl clue as to what is wrong. That is a bug. If the import had failed from within python, it would be fine. But it was an import in C using the C python API and in C, you MUST check the return value and if it is null, call the PyErr functions and exit the function without trying to dereference the null pointer. Your suggested installation procedure is fine for 99% of people. In my environment I have to build from source, so pip or pypy or whatever isn’t an option. I agree it is not high priority, but this is most definitely a bug in the code.

…

Sent from my iPad

On Oct 16, 2018, at 6:05 AM, Ryan P ***@***.***> wrote: Hey David, I'm not sure we actually have a bug here as ImportError actually is a valid built-in exception and it should be raised when certain dependencies are not satisfied. Raised when an import statement fails to find the module definition or when a from ... import fails to find a name that is to be imported. https://docs.python.org/2/library/exceptions.html#exceptions.ImportError I'm not sure how exactly you installed the rc release but might I suggest using pip next time. This will actually call out the missing dependencies before installing the package. pip install --index-url https://test.pypi.org/simple/ confluent-kafka==1.0.0rc1 Looking in indexes: https://test.pypi.org/simple/ Collecting confluent-kafka==1.0.0rc1 Using cached https://test-files.pythonhosted.org/packages/15/10/144f62e629d644c7bda80113f1c63d33ef343b59ebf4c3b746ee33460288/confluent_kafka-1.0.0rc1-cp27-cp27m-macosx_10_6_intel.whl Collecting enum34 (from confluent-kafka==1.0.0rc1) Could not find a version that satisfies the requirement enum34 (from confluent-kafka==1.0.0rc1) (from versions: ) No matching distribution found for enum34 (from confluent-kafka==1.0.0rc1) If you wish to install the dev packages on test.pypi and their dependencies you can use --extra-index-url which will search both test.pyi and pypi the package and its dependencies. pip install --extra-index-url https://test.pypi.org/simple/ confluent-kafka==1.0.0rc1 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[edenhill]

This is definitely a bug, we'll fix it prior to v1.0.0 release. Thank you!