Bump Tornado Version and Remove Workaround from python-telegram-bot#2067 (python-telegram-bot#2494)

Bibo-Joshi · web-flow · commit 9737b1d3c78b · 2021-05-05T20:59:06.000+02:00
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -22,7 +22,7 @@ repos:
           - --rcfile=setup.cfg
         additional_dependencies:
           - certifi
-          - tornado>=5.1
+          - tornado>=6.1
           - APScheduler==3.6.3
           - . # this basically does `pip install -e .`
 -   repo: https://github.com/pre-commit/mirrors-mypy
@@ -33,7 +33,7 @@ repos:
         files: ^telegram/.*\.py$
         additional_dependencies:
           - certifi
-          - tornado>=5.1
+          - tornado>=6.1
           - APScheduler==3.6.3
           - . # this basically does `pip install -e .`
     -   id: mypy
@@ -44,7 +44,7 @@ repos:
           - --follow-imports=silent
         additional_dependencies:
           - certifi
-          - tornado>=5.1
+          - tornado>=6.1
           - APScheduler==3.6.3
           - . # this basically does `pip install -e .`
 -   repo: https://github.com/asottile/pyupgrade
diff --git a/requirements.txt b/requirements.txt
@@ -2,6 +2,6 @@
 # pre-commit hooks for pylint & mypy
 certifi
 # only telegram.ext: # Keep this line here; used in setup(-raw).py
-tornado>=5.1
+tornado>=6.1
 APScheduler==3.6.3
 pytz>=2018.6
diff --git a/telegram/ext/updater.py b/telegram/ext/updater.py
@@ -334,7 +334,7 @@ def start_webhook(
         bootstrap_retries: int = 0,
         webhook_url: str = None,
         allowed_updates: List[str] = None,
-        force_event_loop: bool = False,
+        force_event_loop: bool = None,
         drop_pending_updates: bool = None,
         ip_address: str = None,
     ) -> Optional[Queue]:
@@ -349,15 +349,6 @@ def start_webhook(
             :meth:`start_webhook` now *always* calls :meth:`telegram.Bot.set_webhook`, so pass
             ``webhook_url`` instead of calling ``updater.bot.set_webhook(webhook_url)`` manually.
 
-        Note:
-            Due to an incompatibility of the Tornado library PTB uses for the webhook with Python
-            3.8+ on Windows machines, PTB will attempt to set the event loop to
-            :attr:`asyncio.SelectorEventLoop` and raise an exception, if an incompatible event loop
-            has already been specified. See this `thread`_ for more details. To suppress the
-            exception, set :attr:`force_event_loop` to :obj:`True`.
-
-            .. _thread: https://github.com/tornadoweb/tornado/issues/2608
-
         Args:
             listen (:obj:`str`, optional): IP-Address to listen on. Default ``127.0.0.1``.
             port (:obj:`int`, optional): Port the bot should be listening on. Default ``80``.
@@ -387,8 +378,12 @@ def start_webhook(
                 .. versionadded :: 13.4
             allowed_updates (List[:obj:`str`], optional): Passed to
                 :meth:`telegram.Bot.set_webhook`.
-            force_event_loop (:obj:`bool`, optional): Force using the current event loop. See above
-                note for details. Defaults to :obj:`False`
+            force_event_loop (:obj:`bool`, optional): Legacy parameter formerly used for a
+                workaround on Windows + Python 3.8+. No longer has any effect.
+
+                .. deprecated:: 13.6
+                   Since version 13.6, ``tornade>=6.1`` is required, which resolves the former
+                   issue.
 
         Returns:
             :obj:`Queue`: The update queue that can be filled from the main thread.
@@ -405,6 +400,14 @@ def start_webhook(
                 stacklevel=2,
             )
 
+        if force_event_loop is not None:
+            warnings.warn(
+                'The argument `force_event_loop` of `start_webhook` is deprecated and no longer '
+                'has any effect.',
+                category=TelegramDeprecationWarning,
+                stacklevel=2,
+            )
+
         drop_pending_updates = drop_pending_updates if drop_pending_updates is not None else clean
 
         with self.__lock:
@@ -429,7 +432,6 @@ def start_webhook(
                     webhook_url,
                     allowed_updates,
                     ready=webhook_ready,
-                    force_event_loop=force_event_loop,
                     ip_address=ip_address,
                 )
 
@@ -563,7 +565,6 @@ def _start_webhook(
         webhook_url,
         allowed_updates,
         ready=None,
-        force_event_loop=False,
         ip_address=None,
     ):
         self.logger.debug('Updater thread started (webhook)')
@@ -606,7 +607,7 @@ def _start_webhook(
             ip_address=ip_address,
         )
 
-        self.httpd.serve_forever(force_event_loop=force_event_loop, ready=ready)
+        self.httpd.serve_forever(ready=ready)
 
     @staticmethod
     def _gen_webhook_url(listen: str, port: int, url_path: str) -> str:
diff --git a/telegram/ext/utils/webhookhandler.py b/telegram/ext/utils/webhookhandler.py
@@ -18,10 +18,7 @@
 # along with this program.  If not, see [http://www.gnu.org/licenses/].
 # pylint: disable=C0114
 
-import asyncio
 import logging
-import os
-import sys
 from queue import Queue
 from ssl import SSLContext
 from threading import Event, Lock
@@ -57,11 +54,11 @@ def __init__(
         self.server_lock = Lock()
         self.shutdown_lock = Lock()
 
-    def serve_forever(self, force_event_loop: bool = False, ready: Event = None) -> None:
+    def serve_forever(self, ready: Event = None) -> None:
         with self.server_lock:
+            IOLoop().make_current()
             self.is_running = True
             self.logger.debug('Webhook Server started.')
-            self._ensure_event_loop(force_event_loop=force_event_loop)
             self.loop = IOLoop.current()
             self.http_server.listen(self.port, address=self.listen)
 
@@ -87,54 +84,6 @@ def handle_error(self, request: object, client_address: str) -> None:  # pylint:
             exc_info=True,
         )
 
-    def _ensure_event_loop(self, force_event_loop: bool = False) -> None:
-        """If there's no asyncio event loop set for the current thread - create one."""
-        try:
-            loop = asyncio.get_event_loop()
-            if (
-                not force_event_loop
-                and os.name == 'nt'
-                and sys.version_info >= (3, 8)
-                and isinstance(loop, asyncio.ProactorEventLoop)  # type: ignore[attr-defined]
-            ):
-                raise TypeError(
-                    '`ProactorEventLoop` is incompatible with '
-                    'Tornado. Please switch to `SelectorEventLoop`.'
-                )
-        except RuntimeError:
-            # Python 3.8 changed default asyncio event loop implementation on windows
-            # from SelectorEventLoop to ProactorEventLoop. At the time of this writing
-            # Tornado doesn't support ProactorEventLoop and suggests that end users
-            # change asyncio event loop policy to WindowsSelectorEventLoopPolicy.
-            # https://github.com/tornadoweb/tornado/issues/2608
-            # To avoid changing the global event loop policy, we manually construct
-            # a SelectorEventLoop instance instead of using asyncio.new_event_loop().
-            # Note that the fix is not applied in the main thread, as that can break
-            # user code in even more ways than changing the global event loop policy can,
-            # and because Updater always starts its webhook server in a separate thread.
-            # Ideally, we would want to check that Tornado actually raises the expected
-            # NotImplementedError, but it's not possible to cleanly recover from that
-            # exception in current Tornado version.
-            if (
-                os.name == 'nt'
-                and sys.version_info >= (3, 8)
-                # OS+version check makes hasattr check redundant, but just to be sure
-                and hasattr(asyncio, 'WindowsProactorEventLoopPolicy')
-                and (
-                    isinstance(
-                        asyncio.get_event_loop_policy(),
-                        asyncio.WindowsProactorEventLoopPolicy,  # type: ignore # pylint: disable
-                    )
-                )
-            ):  # pylint: disable=E1101
-                self.logger.debug(
-                    'Applying Tornado asyncio event loop fix for Python 3.8+ on Windows'
-                )
-                loop = asyncio.SelectorEventLoop()
-            else:
-                loop = asyncio.new_event_loop()
-            asyncio.set_event_loop(loop)
-
 
 class WebhookAppClass(tornado.web.Application):
     def __init__(self, webhook_path: str, bot: 'Bot', update_queue: Queue):
diff --git a/tests/test_updater.py b/tests/test_updater.py
@@ -221,101 +221,6 @@ def test_start_webhook_no_warning_or_error_logs(self, caplog, updater, monkeypat
             updater.stop()
         assert not caplog.records
 
-    @pytest.mark.skipif(
-        os.name != 'nt' or sys.version_info < (3, 8),
-        reason='Workaround only relevant on windows with py3.8+',
-    )
-    def test_start_webhook_ensure_event_loop(self, updater, monkeypatch):
-        def serve_forever(self, force_event_loop=False, ready=None):
-            with self.server_lock:
-                self.is_running = True
-                self._ensure_event_loop(force_event_loop=force_event_loop)
-
-                if ready is not None:
-                    ready.set()
-
-        monkeypatch.setattr(WebhookServer, 'serve_forever', serve_forever)
-        monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
-        monkeypatch.setattr(updater.bot, 'delete_webhook', lambda *args, **kwargs: True)
-
-        ip = '127.0.0.1'
-        port = randrange(1024, 49152)  # Select random port
-
-        with set_asyncio_event_loop(None):
-            updater._start_webhook(
-                ip,
-                port,
-                url_path='TOKEN',
-                cert=None,
-                key=None,
-                bootstrap_retries=0,
-                drop_pending_updates=False,
-                webhook_url=None,
-                allowed_updates=None,
-            )
-
-            assert isinstance(asyncio.get_event_loop(), asyncio.SelectorEventLoop)
-
-    @pytest.mark.skipif(
-        os.name != 'nt' or sys.version_info < (3, 8),
-        reason='Workaround only relevant on windows with py3.8+',
-    )
-    def test_start_webhook_force_event_loop_false(self, updater, monkeypatch):
-        monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
-        monkeypatch.setattr(updater.bot, 'delete_webhook', lambda *args, **kwargs: True)
-
-        ip = '127.0.0.1'
-        port = randrange(1024, 49152)  # Select random port
-
-        with set_asyncio_event_loop(asyncio.ProactorEventLoop()):
-            with pytest.raises(TypeError, match='`ProactorEventLoop` is incompatible'):
-                updater._start_webhook(
-                    ip,
-                    port,
-                    url_path='TOKEN',
-                    cert=None,
-                    key=None,
-                    bootstrap_retries=0,
-                    drop_pending_updates=False,
-                    webhook_url=None,
-                    allowed_updates=None,
-                )
-
-    @pytest.mark.skipif(
-        os.name != 'nt' or sys.version_info < (3, 8),
-        reason='Workaround only relevant on windows with py3.8+',
-    )
-    def test_start_webhook_force_event_loop_true(self, updater, monkeypatch):
-        def serve_forever(self, force_event_loop=False, ready=None):
-            with self.server_lock:
-                self.is_running = True
-                self._ensure_event_loop(force_event_loop=force_event_loop)
-
-                if ready is not None:
-                    ready.set()
-
-        monkeypatch.setattr(WebhookServer, 'serve_forever', serve_forever)
-        monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
-        monkeypatch.setattr(updater.bot, 'delete_webhook', lambda *args, **kwargs: True)
-
-        ip = '127.0.0.1'
-        port = randrange(1024, 49152)  # Select random port
-
-        with set_asyncio_event_loop(asyncio.ProactorEventLoop()):
-            updater._start_webhook(
-                ip,
-                port,
-                url_path='TOKEN',
-                cert=None,
-                key=None,
-                bootstrap_retries=0,
-                drop_pending_updates=False,
-                webhook_url=None,
-                allowed_updates=None,
-                force_event_loop=True,
-            )
-            assert isinstance(asyncio.get_event_loop(), asyncio.ProactorEventLoop)
-
     def test_webhook_ssl(self, monkeypatch, updater):
         monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
         monkeypatch.setattr(updater.bot, 'delete_webhook', lambda *args, **kwargs: True)
@@ -450,7 +355,7 @@ def delete_webhook(**kwargs):
         )
         assert self.test_flag is True
 
-    def test_clean_deprecation_warning_webhook(self, recwarn, updater, monkeypatch):
+    def test_deprecation_warnings_start_webhook(self, recwarn, updater, monkeypatch):
         monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
         monkeypatch.setattr(updater.bot, 'delete_webhook', lambda *args, **kwargs: True)
         # prevent api calls from @info decorator when updater.bot.id is used in thread names
@@ -459,11 +364,12 @@ def test_clean_deprecation_warning_webhook(self, recwarn, updater, monkeypatch):
 
         ip = '127.0.0.1'
         port = randrange(1024, 49152)  # Select random port
-        updater.start_webhook(ip, port, clean=True)
+        updater.start_webhook(ip, port, clean=True, force_event_loop=False)
         updater.stop()
-        assert len(recwarn) == 2
+        assert len(recwarn) == 3
         assert str(recwarn[0].message).startswith('Old Handler API')
         assert str(recwarn[1].message).startswith('The argument `clean` of')
+        assert str(recwarn[2].message).startswith('The argument `force_event_loop` of')
 
     def test_clean_deprecation_warning_polling(self, recwarn, updater, monkeypatch):
         monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
