Skip to content

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

Merged
merged 5 commits into from
Apr 21, 2020

Conversation

Amertz08
Copy link
Contributor

Issue #, if available:

Resolves #216

Description of changes:

Change asyncio.Task.current_task invocation to asyncio.current_task to alleviate PendingDeprecationWarning

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Amertz08
Copy link
Contributor Author

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 try/except catch of an AttributeError and then fall back to the compatible version or use and if/else off of sys.version_info. The latter would be more clean imo.

@bhautikpip
Copy link
Contributor

Yes. I also think use of sys.version_info would be more cleaner.

@codecov-io
Copy link

codecov-io commented Apr 11, 2020

Codecov Report

Merging #217 into master will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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              
Impacted Files Coverage Δ
aws_xray_sdk/core/async_context.py 91.52% <100.00%> (+1.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3973f9c...828b0ec. Read the comment docs.

@Amertz08
Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor

@bhautikpip bhautikpip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :)

@bhautikpip bhautikpip merged commit 2d12c83 into aws:master Apr 21, 2020
@Amertz08 Amertz08 deleted the 216-asyncio-pending-deprecation branch April 22, 2020 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

asyncio.Task.current_task PendingDeprecation warning
3 participants