Skip to content
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

Fix deprecation warnings for python 3.10 #262

Merged
merged 1 commit into from
May 22, 2021
Merged

Fix deprecation warnings for python 3.10 #262

merged 1 commit into from
May 22, 2021

Conversation

vaartis
Copy link
Contributor

@vaartis vaartis commented May 20, 2021

asyncio.get_event_loop was marked as deprecated, the documnetation
now refers to asyncio.get_running_loop(1)

asyncio.ensure_future issues a deprecation warning if there is no
running event loop(2), so use asyncio.run which creates and destroys the
loop itself

asyncio.gather issues a warning if run outside of event
loop (i.e. there is no running event loop)(3), so wrap it into an
async def

explicit passing of coroutine objects to asyncio.wait is deprecated
since 3.8(4), so wrap them in asyncio.create_task

plus, add 3.10 to tox.ini

Copy link
Member

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Hi @vaartis, get_running_loop() was not added until Python 3.7 so there needs to be a bigger discussion about Python support before we can move to it. I’ve raised it as a possibility on #260. Let’s see what folks say. Thanks.

Copy link
Member

@andrewgodwin andrewgodwin left a comment

Choose a reason for hiding this comment

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

asgiref is required to work on Python 3.6 due to Django and Flask.

asyncio.get_event_loop was marked as deprecated, the documnetation
now refers to asyncio.get_running_loop([1])

asyncio.ensure_future issues a deprecation warning if there is no
running event loop([2]), so use asyncio.run which creates and destroys the
loop itself

asyncio.gather issues a warning if run outside of event
loop (i.e. there is no running event loop)([3]), so wrap it into an
async def

explicit passing of coroutine objects to asyncio.wait is deprecated
since 3.8([4]), so wrap them in asyncio.create_task

plus, add 3.10 to tox.ini

[1]: https://docs.python.org/3.10/library/asyncio-eventloop.html#asyncio.get_event_loop
[2]: https://docs.python.org/3.10/library/asyncio-future.html#asyncio.ensure_future
[3]: https://docs.python.org/3.10/library/asyncio-task.html#asyncio.gather
[4]: https://docs.python.org/3.10/library/asyncio-task.html#asyncio.wait
@vaartis
Copy link
Contributor Author

vaartis commented May 20, 2021

I have separated things that were added in 3.7 and added a version check. All the changes now use the things added in 3.7 if sys.version_info >= (3, 7), and otherwise use the old versions.

@vaartis vaartis requested a review from andrewgodwin May 20, 2021 19:22
@mgorny
Copy link

mgorny commented May 20, 2021

Thanks for your work on this. I can't say for upstream but I really appreciate the effort you've put to make this neat.

Copy link
Member

@andrewgodwin andrewgodwin left a comment

Choose a reason for hiding this comment

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

Lovely, I much prefer this. I'll merge it once the CI passes.

vaartis added a commit to vaartis/gentoo that referenced this pull request May 20, 2021
Upstream PR: django/asgiref#262

Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
vaartis added a commit to vaartis/gentoo that referenced this pull request May 20, 2021
Upstream PR: django/asgiref#262

Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request May 20, 2021
Upstream PR: django/asgiref#262

Signed-off-by: Ekaterina Vaartis <vaartis@kotobank.ch>
Signed-off-by: Michał Górny <mgorny@gentoo.org>
@andrewgodwin andrewgodwin merged commit 83d4823 into django:main May 22, 2021
@andrewgodwin
Copy link
Member

Thanks for this! Were you testing this against Django, Flask, or something else? Due to the nature of the change I'll have to manually run some tests to make sure we didn't introduce subtle breakage and it'd be nice to know if some of those areas are already covered.

@vaartis
Copy link
Contributor Author

vaartis commented May 22, 2021

Thanks for this! Were you testing this against Django, Flask, or something else? Due to the nature of the change I'll have to manually run some tests to make sure we didn't introduce subtle breakage and it'd be nice to know if some of those areas are already covered.

Django, at least the test suite, did pass. However I didn't test anything on 3.6, only 3.8 and 3.10.

@vaartis vaartis deleted the python-310-deprecations branch May 22, 2021 08:40
@andrewgodwin
Copy link
Member

Django, at least the test suite, did pass. However I didn't test anything on 3.6, only 3.8 and 3.10.

Great, thanks for that. I'm not too worried about 3.6 since this patch barely changes behaviour for it - I'll run a few extra tests with some weird async Django apps I have to make sure and then I'll get this into a release.

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.

None yet

4 participants