-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fixed couple of "leaks" when gc is disabled #979
Conversation
Since I'm using kafka-python in an environment with disabled gc these "leaks" hit me hard.
del X | ||
|
||
# Don't del it here, cause with gc disabled this "leaks" to garbage | ||
# del X |
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.
this one is a vendored library, and I really would prefer fixing upstream. otherwise I'm likely to forget and overwrite when I import a newer version. Have you checked if this is fixed already in six?
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.
Upstream doesn't have this fix, but I've filed a pull request: benjaminp/six#176
kafka/vendor/selectors34.py
Outdated
@@ -93,7 +93,8 @@ def __iter__(self): | |||
return iter(self._selector._fd_to_key) | |||
|
|||
|
|||
class BaseSelector(six.with_metaclass(ABCMeta)): | |||
@six.add_metaclass(ABCMeta) |
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.
same re: vendored library. Can you check with upstream version?
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.
Pull request to vendored library: berkerpeksag/selectors34#7
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.
Upstream merged pull request.
…eter to cleanup WekMethod target preliminary
Sorry for spamming PR with commits. Python is magic sometimes. |
I merged the selectors34 code from upstream separately. I am ok merging the rest, though I would prefer to wait for upstream to merge before changing six. That said, given that the change is mostly trivial I would be ok merging. Can you rebase? |
Merged manually as |
Thank you! |
Bump `six` to `1.11.0`. Most changes do not affect us, but it's good to stay up to date. Also, we will likely start vendoring `enum34` in which case benjaminp/six#178 is needed. Note that this preserves the `kafka-python` customization from #979 which has been submitted upstream as benjaminp/six#176 but not yet merged.
Bump `six` to `1.11.0`. Most changes do not affect us, but it's good to stay up to date. Also, we will likely start vendoring `enum34` in which case benjaminp/six#178 is needed. Note that this preserves the `kafka-python` customization from #979 which has been submitted upstream as benjaminp/six#176 but not yet merged.
Bump `six` to `1.11.0`. Most changes do not affect us, but it's good to stay up to date. Also, we will likely start vendoring `enum34` in which case benjaminp/six#178 is needed. Note that this preserves the `kafka-python` customization from #979 which has been submitted upstream as benjaminp/six#176 but not yet merged.
Since I'm using kafka-python in an environment with disabled gc these
"leaks" hit me hard.