Fix and ban the top-level twisted.internet.reactor imports. (#6835)

extras/qps-bench-server.py

@@ -2,7 +2,7 @@
 from collections import deque
 from time import time
 
-from twisted.internet import reactor
+from twisted.internet import reactor  # noqa: TID253
 from twisted.web.resource import Resource
 from twisted.web.server import NOT_DONE_YET, Site
 

pyproject.toml

@@ -299,6 +299,8 @@ extend-select = [
     "T10",
     # flake8-type-checking
     "TC",
+    # flake8-tidy-imports
+    "TID",
     # pyupgrade
     "UP",
     # pycodestyle warnings
@@ -398,6 +400,11 @@ ignore = [
     "SIM112",
 ]
 
+[tool.ruff.lint.flake8-tidy-imports]
+banned-module-level-imports = [
+    "twisted.internet.reactor",
+]
+
 [tool.ruff.lint.per-file-ignores]
 # Circular import workarounds
 "scrapy/linkextractors/__init__.py" = ["E402"]

tests/CrawlerProcess/reactor_default.py

@@ -1,4 +1,4 @@
-from twisted.internet import reactor  # noqa: F401
+from twisted.internet import reactor  # noqa: F401,TID253
 from twisted.python import log
 
 import scrapy

tests/CrawlerProcess/reactor_default_twisted_reactor_select.py

@@ -1,4 +1,4 @@
-from twisted.internet import reactor  # noqa: F401
+from twisted.internet import reactor  # noqa: F401,TID253
 from twisted.python import log
 
 import scrapy

tests/CrawlerRunner/change_reactor.py

@@ -26,7 +26,7 @@ runner = CrawlerRunner()
 
 d = runner.crawl(NoRequestsSpider)
 
-from twisted.internet import reactor  # noqa: E402
+from twisted.internet import reactor  # noqa: E402,TID253
 
 d.addBoth(callback=lambda _: reactor.stop())
 reactor.run()

tests/CrawlerRunner/ip_address.py

@@ -6,7 +6,6 @@ install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor")
 
 from urllib.parse import urlparse
 
-from twisted.internet import reactor
 from twisted.names import cache, resolve
 from twisted.names import hosts as hostsModule
 from twisted.names.client import Resolver
@@ -44,6 +43,8 @@ class LocalhostSpider(Spider):
 
 
 if __name__ == "__main__":
+    from twisted.internet import reactor
+
     with MockServer() as mock_http_server, MockDNSServer() as mock_dns_server:
         port = urlparse(mock_http_server.http_address).port
         url = f"http://not.a.real.domain:{port}/echo"

tests/mockserver.py

@@ -13,7 +13,7 @@ from typing import TYPE_CHECKING
 from urllib.parse import urlencode
 
 from OpenSSL import SSL
-from twisted.internet import defer, reactor, ssl
+from twisted.internet import defer, ssl
 from twisted.internet.task import deferLater
 from twisted.names import dns, error
 from twisted.names.server import DNSServerFactory
@@ -114,6 +114,8 @@ class LeafResource(resource.Resource):
     isLeaf = True
 
     def deferRequest(self, request, delay, f, *a, **kw):
+        from twisted.internet import reactor
+
         def _cancelrequest(_):
             # silence CancelledError
             d.addErrback(lambda _: None)
@@ -378,6 +380,8 @@ def ssl_context_factory(
 
 
 if __name__ == "__main__":
+    from twisted.internet import reactor
+
     parser = argparse.ArgumentParser()
     parser.add_argument(
         "-t", "--type", type=str, choices=("http", "dns"), default="http"

tests/test_cmdline_crawl_with_pipeline/__init__.py

@@ -2,7 +2,7 @@ import sys
 from pathlib import Path
 from subprocess import PIPE, Popen
 
-from .. import TWISTED_KEEPS_TRACEBACKS
+from tests import TWISTED_KEEPS_TRACEBACKS
 
 
 class TestCmdlineCrawlPipeline:

tests/test_core_downloader.py

@@ -8,7 +8,6 @@ from typing import Any
 
 import OpenSSL.SSL
 import pytest
-from twisted.internet import reactor
 from twisted.internet.defer import Deferred, inlineCallbacks
 from twisted.protocols.policies import WrappingFactory
 from twisted.trial import unittest
@@ -40,6 +39,8 @@ class TestContextFactoryBase(unittest.TestCase):
     context_factory = None
 
     def _listen(self, site):
+        from twisted.internet import reactor
+
         return reactor.listenSSL(
             0,
             site,
@@ -71,6 +72,8 @@ class TestContextFactoryBase(unittest.TestCase):
         client_context_factory: BrowserLikePolicyForHTTPS,
         body: str | None = None,
     ) -> bytes:
+        from twisted.internet import reactor
+
         agent = Agent(reactor, contextFactory=client_context_factory)
         body_producer = _RequestBodyProducer(body.encode()) if body else None
         response: TxResponse = await maybe_deferred_to_future(

tests/test_downloader_handler_twisted_http2.py

@@ -8,7 +8,7 @@ from unittest import mock
 
 import pytest
 from testfixtures import LogCapture
-from twisted.internet import defer, error, reactor
+from twisted.internet import defer, error
 from twisted.web import server
 from twisted.web.error import SchemeNotSupported
 from twisted.web.http import H2_ENABLED
@@ -59,6 +59,8 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base):
 
     @defer.inlineCallbacks
     def test_download_with_maxsize_very_large_file(self):
+        from twisted.internet import reactor
+
         with mock.patch("scrapy.core.http2.stream.logger") as logger:
             request = Request(self.getURL("largechunkedfile"))
 
@@ -207,6 +209,8 @@ class Https2ProxyTestCase(H2DownloadHandlerMixin, TestHttpProxyBase):
     expected_http_proxy_request_body = b"/"
 
     def setUp(self):
+        from twisted.internet import reactor
+
         site = server.Site(UriResource(), timeout=None)
         self.port = reactor.listenSSL(
             0,

tests/test_downloader_handlers.py

@@ -12,7 +12,6 @@ from unittest import mock
 
 import pytest
 from twisted.cred import checkers, credentials, portal
-from twisted.internet import reactor
 from twisted.protocols.ftp import FTPFactory, FTPRealm
 from twisted.trial import unittest
 from w3lib.url import path_to_file_uri
@@ -310,6 +309,8 @@ class TestFTPBase(unittest.TestCase):
     )
 
     def setUp(self):
+        from twisted.internet import reactor
+
         # setup dirs and test file
         self.directory = Path(mkdtemp())
         userdir = self.directory / self.username
@@ -451,6 +452,8 @@ class TestAnonymousFTP(TestFTPBase):
     req_meta = {}
 
     def setUp(self):
+        from twisted.internet import reactor
+
         # setup dir and test file
         self.directory = Path(mkdtemp())
         for filename, content in self.test_files:

tests/test_downloader_handlers_http_base.py

@@ -12,7 +12,7 @@ from unittest import mock
 
 import pytest
 from testfixtures import LogCapture
-from twisted.internet import defer, error, reactor
+from twisted.internet import defer, error
 from twisted.protocols.policies import WrappingFactory
 from twisted.trial import unittest
 from twisted.web import resource, server, static, util
@@ -52,6 +52,8 @@ class ContentLengthHeaderResource(resource.Resource):
 
 class ChunkedResource(resource.Resource):
     def render(self, request):
+        from twisted.internet import reactor
+
         def response():
             request.write(b"chunked ")
             request.write(b"content\n")
@@ -63,6 +65,8 @@ class ChunkedResource(resource.Resource):
 
 class BrokenChunkedResource(resource.Resource):
     def render(self, request):
+        from twisted.internet import reactor
+
         def response():
             request.write(b"chunked ")
             request.write(b"content\n")
@@ -76,6 +80,8 @@ class BrokenChunkedResource(resource.Resource):
 
 class BrokenDownloadResource(resource.Resource):
     def render(self, request):
+        from twisted.internet import reactor
+
         def response():
             request.setHeader(b"Content-Length", b"20")
             request.write(b"partial")
@@ -105,6 +111,8 @@ class EmptyContentTypeHeaderResource(resource.Resource):
 
 class LargeChunkedFileResource(resource.Resource):
     def render(self, request):
+        from twisted.internet import reactor
+
         def response():
             for i in range(1024):
                 request.write(b"x" * 1024)
@@ -133,6 +141,8 @@ class TestHttpBase(unittest.TestCase, ABC):
         raise NotImplementedError
 
     def setUp(self):
+        from twisted.internet import reactor
+
         self.tmpname = Path(mkdtemp())
         (self.tmpname / "file").write_bytes(b"0123456789")
         r = static.File(str(self.tmpname))
@@ -365,6 +375,9 @@ class TestHttp11Base(TestHttpBase):
 
     @defer.inlineCallbacks
     def test_download_with_maxsize_very_large_file(self):
+        from twisted.internet import reactor
+
+        # TODO: the logger check is specific to scrapy.core.downloader.handlers.http11
         with mock.patch("scrapy.core.downloader.handlers.http11.logger") as logger:
             request = Request(self.getURL("largechunkedfile"))
 
@@ -501,6 +514,8 @@ class TestSimpleHttpsBase(unittest.TestCase, ABC):
         raise NotImplementedError
 
     def setUp(self):
+        from twisted.internet import reactor
+
         self.tmpname = Path(mkdtemp())
         (self.tmpname / "file").write_bytes(b"0123456789")
         r = static.File(str(self.tmpname))
@@ -639,6 +654,8 @@ class TestHttpProxyBase(unittest.TestCase, ABC):
         raise NotImplementedError
 
     def setUp(self):
+        from twisted.internet import reactor
+
         site = server.Site(UriResource(), timeout=None)
         wrapper = WrappingFactory(site)
         self.port = reactor.listenTCP(0, wrapper, interface="127.0.0.1")

tests/test_downloadermiddleware_robotstxt.py

@@ -4,7 +4,7 @@ from typing import TYPE_CHECKING
 from unittest import mock
 
 import pytest
-from twisted.internet import error, reactor
+from twisted.internet import error
 from twisted.internet.defer import Deferred, maybeDeferred
 from twisted.python import failure
 from twisted.trial import unittest
@@ -53,6 +53,8 @@ Disallow: /some/randome/page.html
         response = TextResponse("http://site.local/robots.txt", body=ROBOTS)
 
         def return_response(request):
+            from twisted.internet import reactor
+
             deferred = Deferred()
             reactor.callFromThread(deferred.callback, response)
             return deferred
@@ -102,6 +104,8 @@ Disallow: /some/randome/page.html
         )
 
         def return_response(request):
+            from twisted.internet import reactor
+
             deferred = Deferred()
             reactor.callFromThread(deferred.callback, response)
             return deferred
@@ -124,6 +128,8 @@ Disallow: /some/randome/page.html
         response = Response("http://site.local/robots.txt")
 
         def return_response(request):
+            from twisted.internet import reactor
+
             deferred = Deferred()
             reactor.callFromThread(deferred.callback, response)
             return deferred
@@ -145,6 +151,8 @@ Disallow: /some/randome/page.html
         err = error.DNSLookupError("Robotstxt address not found")
 
         def return_failure(request):
+            from twisted.internet import reactor
+
             deferred = Deferred()
             reactor.callFromThread(deferred.errback, failure.Failure(err))
             return deferred
@@ -178,6 +186,8 @@ Disallow: /some/randome/page.html
         self.crawler.settings.set("ROBOTSTXT_OBEY", True)
 
         def ignore_request(request):
+            from twisted.internet import reactor
+
             deferred = Deferred()
             reactor.callFromThread(deferred.errback, failure.Failure(IgnoreRequest()))
             return deferred

tests/test_engine.py

@@ -24,7 +24,7 @@ from urllib.parse import urlparse
 import attr
 from itemadapter import ItemAdapter
 from pydispatch import dispatcher
-from twisted.internet import defer, reactor
+from twisted.internet import defer
 from twisted.trial import unittest
 from twisted.web import server, static, util
 
@@ -130,6 +130,8 @@ class ChangeCloseReasonSpider(MySpider):
 
 
 def start_test_site(debug=False):
+    from twisted.internet import reactor
+
     root_dir = Path(tests_datadir, "test_site")
     r = static.File(str(root_dir))
     r.putChild(b"redirect", util.Redirect(b"/redirected"))
@@ -514,6 +516,8 @@ def test_request_scheduled_signal(caplog):
 
 
 if __name__ == "__main__":
+    from twisted.internet import reactor  # pylint: disable=ungrouped-imports
+
     if len(sys.argv) > 1 and sys.argv[1] == "runserver":
         start_test_site(debug=True)
         reactor.run()

tests/test_http2_client_protocol.py

@@ -13,7 +13,6 @@ from unittest import mock, skipIf
 from urllib.parse import urlencode
 
 import pytest
-from twisted.internet import reactor
 from twisted.internet.defer import (
     CancelledError,
     Deferred,
@@ -209,6 +208,8 @@ class TestHttps2ClientProtocol(TestCase):
 
     @inlineCallbacks
     def setUp(self):
+        from twisted.internet import reactor
+
         # Initialize resource tree
         root = self._init_resource()
         self.site = Site(root, timeout=None)

tests/test_pipeline_media.py

@@ -4,7 +4,6 @@ import warnings
 
 import pytest
 from testfixtures import LogCapture
-from twisted.internet import reactor
 from twisted.internet.defer import Deferred, inlineCallbacks
 from twisted.python.failure import Failure
 from twisted.trial import unittest
@@ -329,6 +328,8 @@ class TestMediaPipeline(TestBaseMediaPipeline):
         rsp1 = Response("http://url")
 
         def rsp1_func():
+            from twisted.internet import reactor
+
             dfd = Deferred().addCallback(_check_downloading)
             reactor.callLater(0.1, dfd.callback, rsp1)
             return dfd

tests/test_utils_signal.py

@@ -3,7 +3,7 @@ import asyncio
 import pytest
 from pydispatch import dispatcher
 from testfixtures import LogCapture
-from twisted.internet import defer, reactor
+from twisted.internet import defer
 from twisted.python.failure import Failure
 from twisted.trial import unittest
 
@@ -65,6 +65,8 @@ class SendCatchLogDeferredTest(TestSendCatchLog):
 
 class SendCatchLogDeferredTest2(SendCatchLogDeferredTest):
     def ok_handler(self, arg, handlers_called):
+        from twisted.internet import reactor
+
         handlers_called.add(self.ok_handler)
         assert arg == "test"
         d = defer.Deferred()
@@ -97,6 +99,8 @@ class SendCatchLogAsyncTest(TestSendCatchLog):
 
 class SendCatchLogAsyncTest2(SendCatchLogAsyncTest):
     def ok_handler(self, arg, handlers_called):
+        from twisted.internet import reactor
+
         handlers_called.add(self.ok_handler)
         assert arg == "test"
         d = defer.Deferred()

tests/test_webclient.py

@@ -12,7 +12,7 @@ from urllib.parse import urlparse
 
 import OpenSSL.SSL
 import pytest
-from twisted.internet import defer, reactor
+from twisted.internet import defer
 from twisted.internet.defer import inlineCallbacks
 from twisted.internet.testing import StringTransport
 from twisted.protocols.policies import WrappingFactory
@@ -205,6 +205,8 @@ class EncodingResource(resource.Resource):
 @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning")
 class TestWebClient(unittest.TestCase):
     def _listen(self, site):
+        from twisted.internet import reactor
+
         return reactor.listenTCP(0, site, interface="127.0.0.1")
 
     def setUp(self):
@@ -318,6 +320,8 @@ class TestWebClient(unittest.TestCase):
         assert b"404 - No Such Resource" in pageData
 
     def testFactoryInfo(self):
+        from twisted.internet import reactor
+
         url = self.getURL("file")
         parsed = urlparse(url)
         factory = client.ScrapyHTTPClientFactory(Request(url))