-
Notifications
You must be signed in to change notification settings - Fork 144
asyncio.Task.current_task PendingDeprecation fix #217
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
asyncio.Task.current_task PendingDeprecation fix #217
Conversation
Yeah so unfortunately simply changing the invocation won't work with 3.5 and 3.6. I think the 2 solutions would be to either do a |
Yes. I also think use of sys.version_info would be more cleaner. |
Codecov Report
@@ Coverage Diff @@
## master #217 +/- ##
==========================================
+ Coverage 84.48% 84.53% +0.04%
==========================================
Files 80 80
Lines 3120 3130 +10
==========================================
+ Hits 2636 2646 +10
Misses 484 484
Continue to review full report at Codecov.
|
Looks like it is all back to green in the test suite. Not sure why the check isn't working. |
|
||
from .context import Context as _Context | ||
|
||
_GTE_PY37 = sys.version_info.minor >= 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.
I would also add condition which checks sys.version_info.major is 2 and 3 because in the case of major version 4.0 above condition would break.
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.
Do we need a guard against 4? I think we do need to make sure its python3. I forgot python 2.7 had asyncio support. So I assumed no python2 code would call this code and if it did it would error for other reasons.
So basically just this.
_GTE_PY37 = sys.version_info.major == 3 and sys.version_info.minor >= 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.
Yep. I think this makes sense.
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.
Looks good :)
Issue #, if available:
Resolves #216
Description of changes:
Change
asyncio.Task.current_task
invocation toasyncio.current_task
to alleviatePendingDeprecationWarning
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.