Repository navigation
ubuntu-22.04 fix: NoneType object has no attribute getcurrent() - #954
farshan-dev wants to merge 1 commit into
Conversation
| return id(greenlet.getcurrent()) | ||
| else: | ||
| return id(gr) | ||
| except: |
There was a problem hiding this comment.
What do you think of making id(gr) the default return value rather than try/except/if/else?
>>> gr = None
>>> id(gr)
140586328307168
Calling id(None) seems to always return something so... why not.
The idea behind these changes LGTM, I'd just suggest to refactor the logic to reduce the complexity of this function. Don't want to pick nit neither. Let me know if that sounds good for you.
There was a problem hiding this comment.
On the other hand I wonder if by forcing the return an identifier even when gr is equal to None we could may find us hidding these symptoms. Symptoms which may be the manifestation of a more pernicious problem.
Please can you give us more details about the context where you face this problem?
Do you have a reproducer?
There was a problem hiding this comment.
if greenlet.getcurrent doesn't exist and if gr is equal to None seems weird.
There was a problem hiding this comment.
That's more or less the scenario you propose to follow.
There was a problem hiding this comment.
As this eventlet module (eventlet.green.thread) is the patched version of the Python thread module I'd argue that maybe, one option, could be to return the result of the native get_ident function. It will return the ‘thread identifier’ of the current thread. IMO, That would more meaningful that returning an integer computed from a None value.
https://docs.python.org/3/library/threading.html#threading.get_ident
But I still continue to think that the observed behavior is the symptom of a problem more deeply rooted.
There was a problem hiding this comment.
thank you for your review, after searching for root of this problem, I found its not related to this package, and its about bobcat keystone package that uses this package. and I tried to fix actual problem and eventlet package works fine.
for reproduce this package install openstack prereqs and set bobcat, then install keystone, and run su -s /bin/sh -c "keystone-manage db_sync", in ubuntu 22.04, you can see error.
thank you again 🙌
There was a problem hiding this comment.
Thanks for your feedback. So, if you agree with abandoning this proposal I think we can close this github pull request and its associated github issue. Do you agree?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #954 +/- ##
=====================================
- Coverage 56% 56% -1%
=====================================
Files 89 89
Lines 9767 9770 +3
Branches 1819 1819
=====================================
- Hits 5485 5476 -9
- Misses 3914 3923 +9
- Partials 368 371 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
related issue: #953