Deleted __del__ as fix for Issue #1218 #1445
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was previously an issue with kafka logging during shutdown, after all of the global variables had been set to `None`. Most of the suggestions to a fix involved using `atexit.register` at the end of the constructor. After deleting the `__del__` function, looking over the rest of this code, and having no trouble with the Kafka producer logging afterwards, I concluded that this fixed the logging issue. Plus, ever since Python 3.4, it generally seems unwise to use __del__.
|
@matthew-mcateer FYI if you include the #1218 directly in your commit message or your pull request description, github will cross-link this with the original issue. You tried to do that by putting it in the PR title, but unfortunately, github only cross-links if the issue number is in the body, not in the title. Additionally, if you just put |
|
I would prefer that we continue calling |
gioele
added a commit
to gioele/kafka-python
that referenced
this pull request
May 1, 2020
Logging in an object destructor leads to crashes like the following ``` Exception ignored in: <bound method KafkaProducer.__del__ of <kafka.producer.kafka.KafkaProducer object at 0x7f0c47309b38>> Traceback (most recent call last): File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 447, in __del__ File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 460, in close File "/usr/local/lib/python3.6/logging/__init__.py", line 1305, in info File "/usr/local/lib/python3.6/logging/__init__.py", line 1546, in isEnabledFor TypeError: '>=' not supported between instances of 'int' and 'NoneType' ``` In issue dpkp#1218 and dpkp#1445, @dpkp made clear that producers should be closed during while they are destructed. This commit does not modify the existing architecture and code structure, and preserves all the logging calls in `close`. At the same time, by "nullifying" the logger, it fixes the crash that is inherent to the use of logging methods during the destruction of an object.
gioele
added a commit
to gioele/kafka-python
that referenced
this pull request
May 1, 2020
Logging in an object destructor leads to crashes like the following ``` Exception ignored in: <bound method KafkaProducer.__del__ of <kafka.producer.kafka.KafkaProducer object at 0x7f0c47309b38>> Traceback (most recent call last): File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 447, in __del__ File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 460, in close File "/usr/local/lib/python3.6/logging/__init__.py", line 1305, in info File "/usr/local/lib/python3.6/logging/__init__.py", line 1546, in isEnabledFor TypeError: '>=' not supported between instances of 'int' and 'NoneType' ``` In issue dpkp#1218 and dpkp#1445, @dpkp made it clear that producers should be closed while they are being destructed. This commit does not modify the existing architecture and code structure, and preserves all the logging calls in `close`. At the same time, by "nullifying" the logger, it fixes the crash that is inherent in the use of logging methods during the destruction of an object.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was previously an issue with kafka logging during shutdown, after all of the global variables had been set to
None. Most of the suggestions to a fix involved usingatexit.registerat the end of the constructor. After deleting the__del__function, looking over the rest of this code, and having no trouble with the Kafka producer logging afterwards, I concluded that this fixed the logging issue. Plus, ever since Python 3.4, it generally seems unwise to use del.