Skip to content

Fix Tornado event_loop warning on Python 3.10 and later#2777

Merged
pquentin merged 4 commits intourllib3:mainfrom
graingert:use-asyncio-run-for-tornado
Nov 10, 2022
Merged

Fix Tornado event_loop warning on Python 3.10 and later#2777
pquentin merged 4 commits intourllib3:mainfrom
graingert:use-asyncio-run-for-tornado

Conversation

@graingert
Copy link
Contributor

@graingert graingert commented Nov 9, 2022

Fixes #2772

@graingert graingert force-pushed the use-asyncio-run-for-tornado branch 7 times, most recently from 33872d0 to 14a2784 Compare November 9, 2022 13:09
@sethmlarson sethmlarson requested a review from pquentin November 9, 2022 13:23
@graingert graingert force-pushed the use-asyncio-run-for-tornado branch 2 times, most recently from 3a8f21a to e374bb4 Compare November 9, 2022 14:01
@pquentin pquentin added the Skip Changelog Pull requests that don't require a changelog entry label Nov 9, 2022
@graingert graingert force-pushed the use-asyncio-run-for-tornado branch 2 times, most recently from acd790f to 1bae78c Compare November 9, 2022 15:26
@graingert graingert force-pushed the use-asyncio-run-for-tornado branch from 1bae78c to 7e4ecbb Compare November 9, 2022 15:28
@graingert graingert mentioned this pull request Nov 9, 2022
Closed
graingert
graingert commented Nov 10, 2022
View reviewed changes
graingert
graingert commented Nov 10, 2022
View reviewed changes
dummyserver/testcase.py
)
cls.server_thread = run_loop_in_thread(cls.io_loop)
with contextlib.ExitStack() as stack:
io_loop = stack.enter_context(run_loop_in_thread())
Copy link
Contributor Author

@graingert graingert Nov 10, 2022

Choose a reason for hiding this comment

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

ExitStack is used so we can use with for the run_loop_in_thread() context manager and close it if anything fails during setup then cls._stack = stack.pop_all() is used to close the context manager later

Copy link
Member

@pquentin pquentin Nov 10, 2022

Choose a reason for hiding this comment

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

So if we had a pytest fixture here instead of setup_class/teardown_class, could we use run_loop_in_thread as a normal context manager? If yes, can we please do that?

Copy link
Contributor Author

@graingert graingert Nov 10, 2022

Choose a reason for hiding this comment

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

yes, can I leave that for another PR?

pquentin
pquentin requested changes Nov 10, 2022
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks. OK for removing ExitStack in another pull request and for leaving from __future__ import annotations. Can you please:

  • Add error:There is no current event loop:DeprecationWarning:tornado.ioloop in setup.cfg as filterwarnings
  • Upgrade to Tornado 6.2
  • Restore the scheme parameter (this will get rid of the confusing ternary expressions)

@graingert graingert force-pushed the use-asyncio-run-for-tornado branch from 7b73405 to 2865721 Compare November 10, 2022 11:59
@graingert
Copy link
Contributor Author

graingert commented Nov 10, 2022

Thanks. OK for removing ExitStack in another pull request and for leaving from __future__ import annotations. Can you please:

* Add `error:There is no current event loop:DeprecationWarning:tornado.ioloop` in setup.cfg as `filterwarnings`

* Upgrade to Tornado 6.2

* Restore the scheme parameter (this will get rid of the confusing ternary expressions)

done!

@graingert graingert force-pushed the use-asyncio-run-for-tornado branch from 2865721 to 3822585 Compare November 10, 2022 12:01
pquentin
pquentin approved these changes Nov 10, 2022
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks you! Looks good to me.

@pquentin pquentin changed the title use asyncio.run for tornado Fix Tornado event_loop warning on Python 3.10 and later Nov 10, 2022
@pquentin pquentin merged commit c2d3739 into urllib3:main Nov 10, 2022
@graingert graingert deleted the use-asyncio-run-for-tornado branch November 10, 2022 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pquentin pquentin pquentin approved these changes

Assignees

No one assigned

Labels

Skip Changelog Pull requests that don't require a changelog entry

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix "DeprecationWarning: There is no current event loop" in test suite

2 participants

@graingert @pquentin