From 1b35260625c3ffec9885265d9ac92771ade67ad9 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 25 Jul 2019 18:18:34 +0500 Subject: [PATCH 01/32] Add a test for downloader middlewares using Deferreds. --- tests/test_downloadermiddleware.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index 6b9a5bee8..1b81ea949 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -1,5 +1,6 @@ from unittest import mock +from twisted.internet.defer import Deferred from twisted.trial.unittest import TestCase from twisted.python.failure import Failure @@ -177,3 +178,31 @@ class ProcessExceptionInvalidOutput(ManagerTestCase): dfd.addBoth(results.append) self.assertIsInstance(results[0], Failure) self.assertIsInstance(results[0].value, _InvalidOutput) + + +class MiddlewareUsingDeferreds(ManagerTestCase): + """Middlewares using Deferreds should work""" + + def test_deferred(self): + resp = Response('http://example.com/index.html') + + class DeferredMiddleware: + def cb(self, result): + return result + + def process_request(self, request, spider): + d = Deferred() + d.addCallback(self.cb) + d.callback(resp) + return d + + self.mwman._add_middleware(DeferredMiddleware()) + req = Request('http://example.com/index.html') + download_func = mock.MagicMock() + dfd = self.mwman.download(download_func, req, self.spider) + results = [] + dfd.addBoth(results.append) + self._wait(dfd) + + self.assertIs(results[0], resp) + self.assertFalse(download_func.called) From 1b437bbe9fa0eb9736f35e510d486805706c783e Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 30 Jul 2019 19:02:16 +0500 Subject: [PATCH 02/32] Install the asyncio reactor on "import scrapy". --- scrapy/__init__.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/scrapy/__init__.py b/scrapy/__init__.py index 230e5cee3..41eaee959 100644 --- a/scrapy/__init__.py +++ b/scrapy/__init__.py @@ -23,6 +23,28 @@ import warnings warnings.filterwarnings('ignore', category=DeprecationWarning, module='twisted') del warnings +# Install twisted asyncio loop +def _install_asyncio_reactor(): + global asyncio_supported + try: + import asyncio + from twisted.internet import asyncioreactor + except ImportError: + pass + else: + from twisted.internet.error import ReactorAlreadyInstalledError + try: + asyncioreactor.install(asyncio.get_event_loop()) + asyncio_supported = True + except ReactorAlreadyInstalledError: + import twisted.internet.reactor + if isinstance(twisted.internet.reactor, + asyncioreactor.AsyncioSelectorReactor): + asyncio_supported = True +asyncio_supported = False +_install_asyncio_reactor() +del _install_asyncio_reactor + # Apply monkey patches to fix issues in external libraries from . import _monkeypatches del _monkeypatches From 9777639533373951652f3865a6321a4dff73246a Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 30 Jul 2019 19:02:59 +0500 Subject: [PATCH 03/32] Run tests using the asyncio reactor. --- pytest.ini | 1 + tests/mockserver.py | 3 +++ tests/requirements-py3.txt | 3 ++- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 33c34b8e8..6c4c21baf 100644 --- a/pytest.ini +++ b/pytest.ini @@ -5,6 +5,7 @@ python_classes= addopts = --assert=plain --doctest-modules + --reactor=asyncio --ignore=docs/_ext --ignore=docs/conf.py --ignore=docs/news.rst diff --git a/tests/mockserver.py b/tests/mockserver.py index 7ebb8bb62..b6aee009a 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -6,6 +6,9 @@ from subprocess import Popen, PIPE from OpenSSL import SSL from six.moves.urllib.parse import urlencode + +import scrapy # needed before importing twisted.internet.reactor + from twisted.web.server import Site, NOT_DONE_YET from twisted.web.resource import Resource from twisted.web.static import File diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index c4bc1f278..26ab08b04 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -4,7 +4,8 @@ mitmproxy; python_version >= '3.6' mitmproxy==3.0.4; python_version < '3.6' pytest pytest-cov -pytest-twisted +#pytest-twisted +-e git+https://github.com/pytest-dev/pytest-twisted@81b91f17#egg=pytest-twisted pytest-xdist sybil testfixtures From 63c3c62305a8c9c52d02ff5524301b7d45eb5724 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 30 Jul 2019 19:45:56 +0500 Subject: [PATCH 04/32] Add utils.deferred_from_coro. --- scrapy/utils/defer.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index c5916c21c..1f6a2584c 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -1,10 +1,14 @@ """ Helper functions for dealing with Twisted deferreds """ +import asyncio +import asyncio.futures +import inspect from twisted.internet import defer, reactor, task from twisted.python import failure +from scrapy import asyncio_supported from scrapy.exceptions import IgnoreRequest @@ -113,3 +117,21 @@ def iter_errback(iterable, errback, *a, **kw): break except Exception: errback(failure.Failure(), *a, **kw) + + +def isfuture(o): + # workaround for Python before 3.5.3 not having asyncio.isfuture + if hasattr(asyncio, 'isfuture'): + return asyncio.isfuture(o) + return isinstance(o, asyncio.futures.Future) + + +def deferred_from_coro(o): + """Converts a coroutine into a Deferred, or returns the object as is if it isn't a coroutine""" + if isinstance(o, defer.Deferred): + return o + if asyncio.iscoroutine(o) or isfuture(o) or inspect.isawaitable(o): + if not asyncio_supported: + raise TypeError('Using coroutines requires installing AsyncioSelectorReactor') + return defer.Deferred.fromFuture(asyncio.ensure_future(o)) + return o From 8d8fbddbde133a94bb8741e48fabec05437a3df9 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 21 Aug 2019 00:07:08 +0500 Subject: [PATCH 05/32] Switch to the released version of pytest-twisted. --- tests/requirements-py3.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index 26ab08b04..2ac434f41 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -4,8 +4,7 @@ mitmproxy; python_version >= '3.6' mitmproxy==3.0.4; python_version < '3.6' pytest pytest-cov -#pytest-twisted --e git+https://github.com/pytest-dev/pytest-twisted@81b91f17#egg=pytest-twisted +pytest-twisted >= 1.11 pytest-xdist sybil testfixtures From b04b541372b219a02bb5fda2cc15cfd9fa1aac66 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 21 Aug 2019 17:14:46 +0500 Subject: [PATCH 06/32] Install the asyncio reactor only in scrapy.cmdline. --- scrapy/__init__.py | 22 ---------------------- scrapy/cmdline.py | 8 +++++++- scrapy/utils/asyncio.py | 26 ++++++++++++++++++++++++++ scrapy/utils/defer.py | 5 +++-- tests/mockserver.py | 2 -- 5 files changed, 36 insertions(+), 27 deletions(-) create mode 100644 scrapy/utils/asyncio.py diff --git a/scrapy/__init__.py b/scrapy/__init__.py index 41eaee959..230e5cee3 100644 --- a/scrapy/__init__.py +++ b/scrapy/__init__.py @@ -23,28 +23,6 @@ import warnings warnings.filterwarnings('ignore', category=DeprecationWarning, module='twisted') del warnings -# Install twisted asyncio loop -def _install_asyncio_reactor(): - global asyncio_supported - try: - import asyncio - from twisted.internet import asyncioreactor - except ImportError: - pass - else: - from twisted.internet.error import ReactorAlreadyInstalledError - try: - asyncioreactor.install(asyncio.get_event_loop()) - asyncio_supported = True - except ReactorAlreadyInstalledError: - import twisted.internet.reactor - if isinstance(twisted.internet.reactor, - asyncioreactor.AsyncioSelectorReactor): - asyncio_supported = True -asyncio_supported = False -_install_asyncio_reactor() -del _install_asyncio_reactor - # Apply monkey patches to fix issues in external libraries from . import _monkeypatches del _monkeypatches diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index 418dc1ac9..d66f0cc2d 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -7,9 +7,9 @@ import inspect import pkg_resources import scrapy -from scrapy.crawler import CrawlerProcess from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError +from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_supported from scrapy.utils.misc import walk_modules from scrapy.utils.project import inside_project, get_project_settings from scrapy.utils.python import garbage_collect @@ -121,6 +121,10 @@ def execute(argv=None, settings=None): settings['EDITOR'] = editor check_deprecated_settings(settings) + # needs to be before _get_commands_dict() as that imports the command modules + # which may import twisted.internet.reactor + install_asyncio_reactor() + inproject = inside_project() cmds = _get_commands_dict(settings, inproject) cmdname = _pop_command_name(argv) @@ -142,6 +146,8 @@ def execute(argv=None, settings=None): opts, args = parser.parse_args(args=argv[1:]) _run_print_help(parser, cmd.process_options, args, opts) + # needs to be after install_asyncio_reactor() as it imports twisted.internet.reactor + from scrapy.crawler import CrawlerProcess cmd.crawler_process = CrawlerProcess(settings) _run_print_help(parser, _run_command, cmd, args, opts) sys.exit(cmd.exitcode) diff --git a/scrapy/utils/asyncio.py b/scrapy/utils/asyncio.py new file mode 100644 index 000000000..e9e3bdd88 --- /dev/null +++ b/scrapy/utils/asyncio.py @@ -0,0 +1,26 @@ +#coding: utf-8 + + +def install_asyncio_reactor(): + """ Tries to install AsyncioSelectorReactor + """ + try: + import asyncio + from twisted.internet import asyncioreactor + except ImportError: + pass + else: + from twisted.internet.error import ReactorAlreadyInstalledError + try: + asyncioreactor.install(asyncio.get_event_loop()) + except ReactorAlreadyInstalledError: + pass + + +def is_asyncio_supported(): + try: + import twisted.internet.reactor + from twisted.internet import asyncioreactor + return isinstance(twisted.internet.reactor, asyncioreactor.AsyncioSelectorReactor) + except ImportError: + return False diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 1f6a2584c..955fc820a 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -8,8 +8,9 @@ import inspect from twisted.internet import defer, reactor, task from twisted.python import failure -from scrapy import asyncio_supported from scrapy.exceptions import IgnoreRequest +from scrapy.utils.asyncio import is_asyncio_supported + def defer_fail(_failure): @@ -131,7 +132,7 @@ def deferred_from_coro(o): if isinstance(o, defer.Deferred): return o if asyncio.iscoroutine(o) or isfuture(o) or inspect.isawaitable(o): - if not asyncio_supported: + if not is_asyncio_supported(): raise TypeError('Using coroutines requires installing AsyncioSelectorReactor') return defer.Deferred.fromFuture(asyncio.ensure_future(o)) return o diff --git a/tests/mockserver.py b/tests/mockserver.py index b6aee009a..d09fbc171 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -7,8 +7,6 @@ from subprocess import Popen, PIPE from OpenSSL import SSL from six.moves.urllib.parse import urlencode -import scrapy # needed before importing twisted.internet.reactor - from twisted.web.server import Site, NOT_DONE_YET from twisted.web.resource import Resource from twisted.web.static import File From 2fbe7d49dc084b7770cc4dc6bbe65eb380b5f498 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 21 Aug 2019 17:16:33 +0500 Subject: [PATCH 07/32] Log asyncio support on spider start. --- scrapy/utils/log.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index e07fb8698..b74b7a4af 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -11,6 +11,7 @@ from twisted.python import log as twisted_log import scrapy from scrapy.settings import Settings from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.asyncio import is_asyncio_supported from scrapy.utils.versions import scrapy_components_versions @@ -148,6 +149,8 @@ def log_scrapy_info(settings): {'versions': ", ".join("%s %s" % (name, version) for name, version in scrapy_components_versions() if name != "Scrapy")}) + if is_asyncio_supported(): + logger.debug("Asyncio support enabled") class StreamLogger(object): From cc19ab5439f20ba6995528542cc064ddab86273c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 22 Aug 2019 18:15:02 +0500 Subject: [PATCH 08/32] Add tests that check asyncio support. --- conftest.py | 4 ++++ tests/test_commands.py | 7 +++++++ tests/test_crawler.py | 20 +++++++++++++++++++- tests/test_utils_asyncio.py | 17 +++++++++++++++++ tox.ini | 6 ++++++ 5 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/test_utils_asyncio.py diff --git a/conftest.py b/conftest.py index d54ce155c..24e31f130 100644 --- a/conftest.py +++ b/conftest.py @@ -27,3 +27,7 @@ def pytest_collection_modifyitems(session, config, items): items[:] = [item for item in items if isinstance(item, Flake8Item)] except ImportError: pass + +@pytest.fixture() +def reactor_pytest(request): + request.cls.reactor_pytest = request.config.getoption("--reactor") diff --git a/tests/test_commands.py b/tests/test_commands.py index 536379170..8aa7ee109 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -9,6 +9,7 @@ from tempfile import mkdtemp from contextlib import contextmanager from threading import Timer +from pytest import mark from twisted.trial import unittest import scrapy @@ -178,6 +179,7 @@ class MiscCommandsTest(CommandTest): self.assertEqual(0, self.call('list')) +@mark.usefixtures('reactor_pytest') class RunSpiderCommandTest(CommandTest): debug_log_spider = """ @@ -295,6 +297,11 @@ class BadSpider(scrapy.Spider): self.assertIn("start_requests", log) self.assertIn("badspider.py", log) + def test_asyncio_supported(self): + if self.reactor_pytest == 'asyncio': + log = self.get_log(self.debug_log_spider) + self.assertIn("DEBUG: Asyncio support enabled", log) + class BenchCommandTest(CommandTest): diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 8eb2389e2..151acb459 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -1,14 +1,16 @@ import logging import warnings +from pytest import raises, mark +from testfixtures import LogCapture from twisted.internet import defer from twisted.trial import unittest -from pytest import raises import scrapy from scrapy.crawler import Crawler, CrawlerRunner, CrawlerProcess from scrapy.settings import Settings, default_settings from scrapy.spiderloader import SpiderLoader +from scrapy.utils.asyncio import is_asyncio_supported from scrapy.utils.log import configure_logging, get_scrapy_root_handler from scrapy.utils.spider import DefaultSpider from scrapy.utils.misc import load_object @@ -203,6 +205,15 @@ class NoRequestsSpider(scrapy.Spider): return [] +class AsyncioSpider(scrapy.Spider): + name = 'asyncio' + + def start_requests(self): + self.logger.info('Asyncio support: %s', is_asyncio_supported()) + return [] + + +@mark.usefixtures('reactor_pytest') class CrawlerRunnerHasSpider(unittest.TestCase): @defer.inlineCallbacks @@ -245,3 +256,10 @@ class CrawlerRunnerHasSpider(unittest.TestCase): yield runner.crawl(NoRequestsSpider) self.assertEqual(runner.bootstrap_failed, True) + + @defer.inlineCallbacks + def test_asyncio_supported(self): + runner = CrawlerRunner() + with LogCapture() as log: + yield runner.crawl(AsyncioSpider) + log.check_present(('asyncio', 'INFO', 'Asyncio support: %s' % (self.reactor_pytest == 'asyncio'))) diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py new file mode 100644 index 000000000..e34d3002a --- /dev/null +++ b/tests/test_utils_asyncio.py @@ -0,0 +1,17 @@ +from unittest import TestCase + +from pytest import mark + +from scrapy.utils.asyncio import is_asyncio_supported, install_asyncio_reactor + + +@mark.usefixtures('reactor_pytest') +class AsyncioTest(TestCase): + + def test_is_asyncio_supported(self): + # the result should depend only on the pytest --reactor argument + self.assertEquals(is_asyncio_supported(), self.reactor_pytest == 'asyncio') + + def test_install_asyncio_reactor(self): + # this should do nothing + install_asyncio_reactor() diff --git a/tox.ini b/tox.ini index fd75d18e2..844956e5f 100644 --- a/tox.ini +++ b/tox.ini @@ -106,3 +106,9 @@ deps = {[testenv]deps} reppy robotexclusionrulesparser + +[testenv:py38-no-asyncio] +basepython = python3.8 +deps = {[testenv]deps} +commands = + py.test --cov=scrapy --cov-report= --reactor=default {posargs:scrapy tests} From f41c2f3874d2f9deac365d633a71f032e1339e3c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 22 Aug 2019 21:24:30 +0500 Subject: [PATCH 09/32] Add py38-no-asyncio to Travis. --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 9f477e860..fdf40fdf1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,6 +25,8 @@ matrix: python: 3.8 - env: TOXENV=py38-extra-deps python: 3.8 + - env: TOXENV=py38-no-asyncio + python: 3.8 - env: TOXENV=docs python: 3.6 install: From 3ba25ccbd3b456024b1d350645407557c81d73c7 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 8 Nov 2019 00:09:28 +0500 Subject: [PATCH 10/32] Don't use asyncio.iscoroutine, as it is True for generators. --- scrapy/utils/defer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 955fc820a..30163d2fb 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -131,7 +131,7 @@ def deferred_from_coro(o): """Converts a coroutine into a Deferred, or returns the object as is if it isn't a coroutine""" if isinstance(o, defer.Deferred): return o - if asyncio.iscoroutine(o) or isfuture(o) or inspect.isawaitable(o): + if isfuture(o) or inspect.isawaitable(o): if not is_asyncio_supported(): raise TypeError('Using coroutines requires installing AsyncioSelectorReactor') return defer.Deferred.fromFuture(asyncio.ensure_future(o)) From 794cf71806a94ba68238a93e75c396f355159ab5 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 14 Nov 2019 13:27:21 +0500 Subject: [PATCH 11/32] Fix or ignore flake8 problems. --- pytest.ini | 2 ++ scrapy/cmdline.py | 2 +- scrapy/utils/asyncio.py | 3 --- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pytest.ini b/pytest.ini index 6c4c21baf..8b97237c3 100644 --- a/pytest.ini +++ b/pytest.ini @@ -116,6 +116,7 @@ flake8-ignore = scrapy/spiders/feed.py E501 E261 scrapy/spiders/sitemap.py E501 # scrapy/utils + scrapy/utils/asyncio.py E501 scrapy/utils/benchserver.py E501 scrapy/utils/conf.py E402 E502 E501 scrapy/utils/console.py E261 E306 E305 @@ -227,6 +228,7 @@ flake8-ignore = tests/test_spidermiddleware_output_chain.py E501 W293 E226 tests/test_spidermiddleware_referer.py E501 F841 E125 E201 E261 E124 E501 E241 E121 tests/test_squeues.py E501 E701 E741 + tests/test_utils_asyncio.py E501 tests/test_utils_conf.py E501 E303 E128 tests/test_utils_curl.py E501 tests/test_utils_datatypes.py E402 E501 E305 diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index d66f0cc2d..213e99bc0 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -9,7 +9,7 @@ import pkg_resources import scrapy from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError -from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_supported +from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.misc import walk_modules from scrapy.utils.project import inside_project, get_project_settings from scrapy.utils.python import garbage_collect diff --git a/scrapy/utils/asyncio.py b/scrapy/utils/asyncio.py index e9e3bdd88..f732774f1 100644 --- a/scrapy/utils/asyncio.py +++ b/scrapy/utils/asyncio.py @@ -1,6 +1,3 @@ -#coding: utf-8 - - def install_asyncio_reactor(): """ Tries to install AsyncioSelectorReactor """ From c079d5002bae90dbd85bee1f61fdc359b9f39d29 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 21 Nov 2019 23:40:16 +0500 Subject: [PATCH 12/32] Run tests without asyncio support by default, add py35-asyncio and py38-asyncio envs. --- pytest.ini | 1 - tox.ini | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pytest.ini b/pytest.ini index 8b97237c3..336ef041d 100644 --- a/pytest.ini +++ b/pytest.ini @@ -5,7 +5,6 @@ python_classes= addopts = --assert=plain --doctest-modules - --reactor=asyncio --ignore=docs/_ext --ignore=docs/conf.py --ignore=docs/news.rst diff --git a/tox.ini b/tox.ini index 844956e5f..a4edae439 100644 --- a/tox.ini +++ b/tox.ini @@ -107,8 +107,14 @@ deps = reppy robotexclusionrulesparser -[testenv:py38-no-asyncio] +[testenv:py35-asyncio] +basepython = python3.5 +deps = {[testenv]deps} +commands = + py.test --cov=scrapy --cov-report= --reactor=asyncio {posargs:scrapy tests} + +[testenv:py38-asyncio] basepython = python3.8 deps = {[testenv]deps} commands = - py.test --cov=scrapy --cov-report= --reactor=default {posargs:scrapy tests} + py.test --cov=scrapy --cov-report= --reactor=asyncio {posargs:scrapy tests} From ed34ce14c0c06d4539d4fdeb0ad014f4e6fb5b94 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 4 Dec 2019 21:32:16 +0500 Subject: [PATCH 13/32] Add the ASYNCIO_SUPPORT setting, reshuffle other logic accordingly. --- docs/topics/settings.rst | 25 +++++++++++++++++++++++++ scrapy/cmdline.py | 8 ++------ scrapy/commands/crawl.py | 3 +++ scrapy/commands/runspider.py | 3 +++ scrapy/settings/default_settings.py | 2 ++ scrapy/utils/asyncio.py | 2 +- scrapy/utils/defer.py | 17 +++++++++++------ scrapy/utils/log.py | 11 ++++++++--- tests/test_commands.py | 13 +++++++------ tests/test_crawler.py | 24 +++++++++++++++++++++--- tests/test_utils_asyncio.py | 6 +++--- 11 files changed, 86 insertions(+), 28 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index a1d15a760..43f59f7cc 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -160,6 +160,31 @@ to any particular component. In that case the module of that component will be shown, typically an extension, middleware or pipeline. It also means that the component must be enabled in order for the setting to have any effect. +.. setting:: ASYNCIO_SUPPORT + +ASYNCIO_SUPPORT +--------------- + +Default: ``False`` + +Whether to support ``async def`` methods and callbacks which use code that +requires an asyncio loop. + +If an ``async def`` coroutine doesn't require the asyncio loop, it will work +even if this is set to ``False``. Coroutines that require the asyncio loop may +silently fail to run or raise errors unless this is set to ``True``. + +When this option is set to ``True``, Scrapy will require +:class:`~twisted.internet.asyncioreactor.AsyncioSelectorReactor`. It will +install this reactor if no reactor is installed yet, such as when using the +``scrapy`` script or :class:`~scrapy.crawler.CrawlerProcess`. If you are using +:class:`~scrapy.crawler.CrawlerRunner`, you need to install the correct reactor +manually. + +The default value for this option is currently ``False`` to maintain backward +compatibility and avoid possible problems caused by using a different Twisted +reactor. + .. setting:: AWS_ACCESS_KEY_ID AWS_ACCESS_KEY_ID diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index 213e99bc0..ce030cf75 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -9,7 +9,6 @@ import pkg_resources import scrapy from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError -from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.misc import walk_modules from scrapy.utils.project import inside_project, get_project_settings from scrapy.utils.python import garbage_collect @@ -121,10 +120,6 @@ def execute(argv=None, settings=None): settings['EDITOR'] = editor check_deprecated_settings(settings) - # needs to be before _get_commands_dict() as that imports the command modules - # which may import twisted.internet.reactor - install_asyncio_reactor() - inproject = inside_project() cmds = _get_commands_dict(settings, inproject) cmdname = _pop_command_name(argv) @@ -146,7 +141,8 @@ def execute(argv=None, settings=None): opts, args = parser.parse_args(args=argv[1:]) _run_print_help(parser, cmd.process_options, args, opts) - # needs to be after install_asyncio_reactor() as it imports twisted.internet.reactor + # needs to be after cmd.process_options() as it imports twisted.internet.reactor + # while commands may want to install the asyncio reactor from scrapy.crawler import CrawlerProcess cmd.crawler_process = CrawlerProcess(settings) _run_print_help(parser, _run_command, cmd, args, opts) diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index 8093fd402..e2e69be49 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -1,5 +1,6 @@ import os from scrapy.commands import ScrapyCommand +from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.conf import arglist_to_dict from scrapy.utils.python import without_none_values from scrapy.exceptions import UsageError @@ -26,6 +27,8 @@ class Command(ScrapyCommand): def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) + if self.settings.getbool('ASYNCIO_SUPPORT'): + install_asyncio_reactor() try: opts.spargs = arglist_to_dict(opts.spargs) except ValueError: diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 57d8471ca..ebd4eb620 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -2,6 +2,7 @@ import sys import os from importlib import import_module +from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.spider import iter_spider_classes from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError @@ -50,6 +51,8 @@ class Command(ScrapyCommand): def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) + if self.settings.getbool('ASYNCIO_SUPPORT'): + install_asyncio_reactor() try: opts.spargs = arglist_to_dict(opts.spargs) except ValueError: diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 5c9678c01..c9097bd1f 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -19,6 +19,8 @@ from os.path import join, abspath, dirname AJAXCRAWL_ENABLED = False +ASYNCIO_SUPPORT = False + AUTOTHROTTLE_ENABLED = False AUTOTHROTTLE_DEBUG = False AUTOTHROTTLE_MAX_DELAY = 60.0 diff --git a/scrapy/utils/asyncio.py b/scrapy/utils/asyncio.py index f732774f1..b5d5f92d9 100644 --- a/scrapy/utils/asyncio.py +++ b/scrapy/utils/asyncio.py @@ -14,7 +14,7 @@ def install_asyncio_reactor(): pass -def is_asyncio_supported(): +def is_asyncio_reactor_installed(): try: import twisted.internet.reactor from twisted.internet import asyncioreactor diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 30163d2fb..3b7ef75ab 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -9,8 +9,7 @@ from twisted.internet import defer, reactor, task from twisted.python import failure from scrapy.exceptions import IgnoreRequest -from scrapy.utils.asyncio import is_asyncio_supported - +from scrapy.utils.asyncio import is_asyncio_reactor_installed def defer_fail(_failure): @@ -127,12 +126,18 @@ def isfuture(o): return isinstance(o, asyncio.futures.Future) -def deferred_from_coro(o): +def deferred_from_coro(o, asyncio_enabled=False): """Converts a coroutine into a Deferred, or returns the object as is if it isn't a coroutine""" if isinstance(o, defer.Deferred): return o if isfuture(o) or inspect.isawaitable(o): - if not is_asyncio_supported(): - raise TypeError('Using coroutines requires installing AsyncioSelectorReactor') - return defer.Deferred.fromFuture(asyncio.ensure_future(o)) + if not asyncio_enabled: + # wrapping the coroutine directly into a Deferred, this doesn't work correctly with coroutines + # that use asyncio, e.g. "await asyncio.sleep(1)" + return defer.ensureDeferred(o) + else: + # wrapping the coroutine into a Future and then into a Deferred, this requires AsyncioSelectorReactor + if not is_asyncio_reactor_installed(): + raise TypeError('Using coroutines requires installing AsyncioSelectorReactor') + return defer.Deferred.fromFuture(asyncio.ensure_future(o)) return o diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index b74b7a4af..8c56cfa42 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -11,7 +11,7 @@ from twisted.python import log as twisted_log import scrapy from scrapy.settings import Settings from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.asyncio import is_asyncio_supported +from scrapy.utils.asyncio import is_asyncio_reactor_installed from scrapy.utils.versions import scrapy_components_versions @@ -149,8 +149,13 @@ def log_scrapy_info(settings): {'versions': ", ".join("%s %s" % (name, version) for name, version in scrapy_components_versions() if name != "Scrapy")}) - if is_asyncio_supported(): - logger.debug("Asyncio support enabled") + if settings.getbool('ASYNCIO_SUPPORT'): + if is_asyncio_reactor_installed(): + logger.debug("Asyncio support enabled") + else: + logger.error("ASYNCIO_SUPPORT is on but the Twisted asyncio " + "reactor is not installed, this is not supported " + "and asyncio coroutines will not work.") class StreamLogger(object): diff --git a/tests/test_commands.py b/tests/test_commands.py index 8aa7ee109..3b64bfa23 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -9,7 +9,6 @@ from tempfile import mkdtemp from contextlib import contextmanager from threading import Timer -from pytest import mark from twisted.trial import unittest import scrapy @@ -179,7 +178,6 @@ class MiscCommandsTest(CommandTest): self.assertEqual(0, self.call('list')) -@mark.usefixtures('reactor_pytest') class RunSpiderCommandTest(CommandTest): debug_log_spider = """ @@ -297,10 +295,13 @@ class BadSpider(scrapy.Spider): self.assertIn("start_requests", log) self.assertIn("badspider.py", log) - def test_asyncio_supported(self): - if self.reactor_pytest == 'asyncio': - log = self.get_log(self.debug_log_spider) - self.assertIn("DEBUG: Asyncio support enabled", log) + def test_asyncio_support_true(self): + log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_SUPPORT=True']) + self.assertIn("DEBUG: Asyncio support enabled", log) + + def test_asyncio_support_false(self): + log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_SUPPORT=False']) + self.assertNotIn("DEBUG: Asyncio support enabled", log) class BenchCommandTest(CommandTest): diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 151acb459..3ac45ca1d 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -10,7 +10,7 @@ import scrapy from scrapy.crawler import Crawler, CrawlerRunner, CrawlerProcess from scrapy.settings import Settings, default_settings from scrapy.spiderloader import SpiderLoader -from scrapy.utils.asyncio import is_asyncio_supported +from scrapy.utils.asyncio import is_asyncio_reactor_installed from scrapy.utils.log import configure_logging, get_scrapy_root_handler from scrapy.utils.spider import DefaultSpider from scrapy.utils.misc import load_object @@ -209,7 +209,7 @@ class AsyncioSpider(scrapy.Spider): name = 'asyncio' def start_requests(self): - self.logger.info('Asyncio support: %s', is_asyncio_supported()) + self.logger.info('Asyncio support: %s', is_asyncio_reactor_installed()) return [] @@ -258,7 +258,25 @@ class CrawlerRunnerHasSpider(unittest.TestCase): self.assertEqual(runner.bootstrap_failed, True) @defer.inlineCallbacks - def test_asyncio_supported(self): + def test_crawler_process_asyncio_supported_true(self): + with LogCapture(level=logging.DEBUG) as log: + runner = CrawlerProcess(settings={'ASYNCIO_SUPPORT': True}) + yield runner.crawl(NoRequestsSpider) + if self.reactor_pytest == 'asyncio': + self.assertIn("Asyncio support enabled", str(log)) + else: + self.assertNotIn("Asyncio support enabled", str(log)) + self.assertIn("ASYNCIO_SUPPORT is on but the Twisted asyncio reactor is not installed", str(log)) + + @defer.inlineCallbacks + def test_crawler_process_asyncio_supported_false(self): + runner = CrawlerProcess(settings={'ASYNCIO_SUPPORT': False}) + with LogCapture(level=logging.DEBUG) as log: + yield runner.crawl(NoRequestsSpider) + self.assertNotIn("Asyncio support enabled", str(log)) + + @defer.inlineCallbacks + def test_crawler_runner_asyncio_supported(self): runner = CrawlerRunner() with LogCapture() as log: yield runner.crawl(AsyncioSpider) diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index e34d3002a..a6ba24876 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -2,15 +2,15 @@ from unittest import TestCase from pytest import mark -from scrapy.utils.asyncio import is_asyncio_supported, install_asyncio_reactor +from scrapy.utils.asyncio import is_asyncio_reactor_installed, install_asyncio_reactor @mark.usefixtures('reactor_pytest') class AsyncioTest(TestCase): - def test_is_asyncio_supported(self): + def test_is_asyncio_reactor_installed(self): # the result should depend only on the pytest --reactor argument - self.assertEquals(is_asyncio_supported(), self.reactor_pytest == 'asyncio') + self.assertEquals(is_asyncio_reactor_installed(), self.reactor_pytest == 'asyncio') def test_install_asyncio_reactor(self): # this should do nothing From 97fb61cec846641eb1c8e224ae24e55558746f4f Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 4 Dec 2019 21:53:07 +0500 Subject: [PATCH 14/32] Move an import to postpone another "import twisted.internet.reactor". --- scrapy/commands/shell.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index e05084272..7516e2aba 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -6,7 +6,6 @@ See documentation in docs/topics/shell.rst from threading import Thread from scrapy.commands import ScrapyCommand -from scrapy.shell import Shell from scrapy.http import Request from scrapy.utils.spider import spidercls_for_request, DefaultSpider from scrapy.utils.url import guess_scheme @@ -70,6 +69,8 @@ class Command(ScrapyCommand): self._start_crawler_thread() + # moved from the top-level because it imports twisted.internet.reactor + from scrapy.shell import Shell shell = Shell(crawler, update_vars=self.update_vars, code=opts.code) shell.start(url=url, redirect=not opts.no_redirect) From 0b9f29215ff7f81203efbbc25b8e9cf3e9719920 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 4 Dec 2019 22:06:35 +0500 Subject: [PATCH 15/32] Update .travis.yml. --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index fdf40fdf1..98dab01f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,6 +17,8 @@ matrix: python: 3.5 - env: TOXENV=py35-pinned python: 3.5 + - env: TOXENV=py35-asyncio + python: 3.5 - env: TOXENV=py36 python: 3.6 - env: TOXENV=py37 @@ -25,7 +27,7 @@ matrix: python: 3.8 - env: TOXENV=py38-extra-deps python: 3.8 - - env: TOXENV=py38-no-asyncio + - env: TOXENV=py38-asyncio python: 3.8 - env: TOXENV=docs python: 3.6 From 3560123090c1660fcfad7c5e7da08c9af503940f Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 5 Dec 2019 19:06:51 +0500 Subject: [PATCH 16/32] Rename ASYNCIO_SUPPORT to ASYNCIO_ENABLED. --- docs/topics/settings.rst | 4 ++-- scrapy/commands/crawl.py | 2 +- scrapy/commands/runspider.py | 2 +- scrapy/settings/default_settings.py | 2 +- scrapy/utils/log.py | 4 ++-- tests/test_commands.py | 8 ++++---- tests/test_crawler.py | 10 +++++----- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 43f59f7cc..5cbf7450e 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -160,9 +160,9 @@ to any particular component. In that case the module of that component will be shown, typically an extension, middleware or pipeline. It also means that the component must be enabled in order for the setting to have any effect. -.. setting:: ASYNCIO_SUPPORT +.. setting:: ASYNCIO_ENABLED -ASYNCIO_SUPPORT +ASYNCIO_ENABLED --------------- Default: ``False`` diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index e2e69be49..b50761e4a 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -27,7 +27,7 @@ class Command(ScrapyCommand): def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) - if self.settings.getbool('ASYNCIO_SUPPORT'): + if self.settings.getbool('ASYNCIO_ENABLED'): install_asyncio_reactor() try: opts.spargs = arglist_to_dict(opts.spargs) diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index ebd4eb620..bfe844eb5 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -51,7 +51,7 @@ class Command(ScrapyCommand): def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) - if self.settings.getbool('ASYNCIO_SUPPORT'): + if self.settings.getbool('ASYNCIO_ENABLED'): install_asyncio_reactor() try: opts.spargs = arglist_to_dict(opts.spargs) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index c9097bd1f..153b8037a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -19,7 +19,7 @@ from os.path import join, abspath, dirname AJAXCRAWL_ENABLED = False -ASYNCIO_SUPPORT = False +ASYNCIO_ENABLED = False AUTOTHROTTLE_ENABLED = False AUTOTHROTTLE_DEBUG = False diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 8c56cfa42..0fe3d1549 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -149,11 +149,11 @@ def log_scrapy_info(settings): {'versions': ", ".join("%s %s" % (name, version) for name, version in scrapy_components_versions() if name != "Scrapy")}) - if settings.getbool('ASYNCIO_SUPPORT'): + if settings.getbool('ASYNCIO_ENABLED'): if is_asyncio_reactor_installed(): logger.debug("Asyncio support enabled") else: - logger.error("ASYNCIO_SUPPORT is on but the Twisted asyncio " + logger.error("ASYNCIO_ENABLED is on but the Twisted asyncio " "reactor is not installed, this is not supported " "and asyncio coroutines will not work.") diff --git a/tests/test_commands.py b/tests/test_commands.py index 3b64bfa23..197d80217 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -295,12 +295,12 @@ class BadSpider(scrapy.Spider): self.assertIn("start_requests", log) self.assertIn("badspider.py", log) - def test_asyncio_support_true(self): - log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_SUPPORT=True']) + def test_asyncio_enabled_true(self): + log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_ENABLED=True']) self.assertIn("DEBUG: Asyncio support enabled", log) - def test_asyncio_support_false(self): - log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_SUPPORT=False']) + def test_asyncio_enabled_false(self): + log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_ENABLED=False']) self.assertNotIn("DEBUG: Asyncio support enabled", log) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 9410b0e7a..05909d995 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -262,19 +262,19 @@ class CrawlerRunnerHasSpider(unittest.TestCase): self.assertEqual(runner.bootstrap_failed, True) @defer.inlineCallbacks - def test_crawler_process_asyncio_supported_true(self): + def test_crawler_process_asyncio_enabled_true(self): with LogCapture(level=logging.DEBUG) as log: - runner = CrawlerProcess(settings={'ASYNCIO_SUPPORT': True}) + runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': True}) yield runner.crawl(NoRequestsSpider) if self.reactor_pytest == 'asyncio': self.assertIn("Asyncio support enabled", str(log)) else: self.assertNotIn("Asyncio support enabled", str(log)) - self.assertIn("ASYNCIO_SUPPORT is on but the Twisted asyncio reactor is not installed", str(log)) + self.assertIn("ASYNCIO_ENABLED is on but the Twisted asyncio reactor is not installed", str(log)) @defer.inlineCallbacks - def test_crawler_process_asyncio_supported_false(self): - runner = CrawlerProcess(settings={'ASYNCIO_SUPPORT': False}) + def test_crawler_process_asyncio_enabled_false(self): + runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': False}) with LogCapture(level=logging.DEBUG) as log: yield runner.crawl(NoRequestsSpider) self.assertNotIn("Asyncio support enabled", str(log)) From 69cd2e247efe1823ab188eacb241b9e5596a879c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Sat, 7 Dec 2019 00:09:53 +0500 Subject: [PATCH 17/32] Move a bunch of "from twisted.internet import reactor" inside functions. --- scrapy/cmdline.py | 4 +--- scrapy/commands/shell.py | 3 +-- scrapy/crawler.py | 7 ++++++- scrapy/shell.py | 3 ++- scrapy/utils/defer.py | 4 +++- scrapy/utils/ossignal.py | 3 +-- scrapy/utils/reactor.py | 4 +++- 7 files changed, 17 insertions(+), 11 deletions(-) diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index 3c2efe58f..69e917004 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -6,6 +6,7 @@ import inspect import pkg_resources import scrapy +from scrapy.crawler import CrawlerProcess from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError from scrapy.utils.misc import walk_modules @@ -140,9 +141,6 @@ def execute(argv=None, settings=None): opts, args = parser.parse_args(args=argv[1:]) _run_print_help(parser, cmd.process_options, args, opts) - # needs to be after cmd.process_options() as it imports twisted.internet.reactor - # while commands may want to install the asyncio reactor - from scrapy.crawler import CrawlerProcess cmd.crawler_process = CrawlerProcess(settings) _run_print_help(parser, _run_command, cmd, args, opts) sys.exit(cmd.exitcode) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 7516e2aba..d44a32d5f 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -7,6 +7,7 @@ from threading import Thread from scrapy.commands import ScrapyCommand from scrapy.http import Request +from scrapy.shell import Shell from scrapy.utils.spider import spidercls_for_request, DefaultSpider from scrapy.utils.url import guess_scheme @@ -69,8 +70,6 @@ class Command(ScrapyCommand): self._start_crawler_thread() - # moved from the top-level because it imports twisted.internet.reactor - from scrapy.shell import Shell shell = Shell(crawler, update_vars=self.update_vars, code=opts.code) shell.start(url=url, redirect=not opts.no_redirect) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 6c7eb737b..450260004 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -3,7 +3,7 @@ import pprint import signal import warnings -from twisted.internet import reactor, defer +from twisted.internet import defer from zope.interface.verify import verifyClass, DoesNotImplement from scrapy import Spider @@ -261,6 +261,7 @@ class CrawlerProcess(CrawlerRunner): log_scrapy_info(self.settings) def _signal_shutdown(self, signum, _): + from twisted.internet import reactor install_shutdown_handlers(self._signal_kill) signame = signal_names[signum] logger.info("Received %(signame)s, shutting down gracefully. Send again to force ", @@ -268,6 +269,7 @@ class CrawlerProcess(CrawlerRunner): reactor.callFromThread(self._graceful_stop_reactor) def _signal_kill(self, signum, _): + from twisted.internet import reactor install_shutdown_handlers(signal.SIG_IGN) signame = signal_names[signum] logger.info('Received %(signame)s twice, forcing unclean shutdown', @@ -286,6 +288,7 @@ class CrawlerProcess(CrawlerRunner): :param boolean stop_after_crawl: stop or not the reactor when all crawlers have finished """ + from twisted.internet import reactor if stop_after_crawl: d = self.join() # Don't start the reactor if the deferreds are already fired @@ -300,6 +303,7 @@ class CrawlerProcess(CrawlerRunner): reactor.run(installSignalHandlers=False) # blocking call def _get_dns_resolver(self): + from twisted.internet import reactor if self.settings.getbool('DNSCACHE_ENABLED'): cache_size = self.settings.getint('DNSCACHE_SIZE') else: @@ -316,6 +320,7 @@ class CrawlerProcess(CrawlerRunner): return d def _stop_reactor(self, _=None): + from twisted.internet import reactor try: reactor.stop() except RuntimeError: # raised if already stopped or in shutdown stage diff --git a/scrapy/shell.py b/scrapy/shell.py index a649d555f..a23b04df9 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -7,7 +7,7 @@ import os import signal import warnings -from twisted.internet import reactor, threads, defer +from twisted.internet import threads, defer from twisted.python import threadable from w3lib.url import any_to_uri @@ -98,6 +98,7 @@ class Shell(object): return spider def fetch(self, request_or_url, spider=None, redirect=True, **kwargs): + from twisted.internet import reactor if isinstance(request_or_url, Request): request = request_or_url else: diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 3b7ef75ab..6a91776c7 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -5,7 +5,7 @@ import asyncio import asyncio.futures import inspect -from twisted.internet import defer, reactor, task +from twisted.internet import defer, task from twisted.python import failure from scrapy.exceptions import IgnoreRequest @@ -19,6 +19,7 @@ def defer_fail(_failure): It delays by 100ms so reactor has a chance to go through readers and writers before attending pending delayed calls, so do not set delay to zero. """ + from twisted.internet import reactor d = defer.Deferred() reactor.callLater(0.1, d.errback, _failure) return d @@ -31,6 +32,7 @@ def defer_succeed(result): It delays by 100ms so reactor has a chance to go trough readers and writers before attending pending delayed calls, so do not set delay to zero. """ + from twisted.internet import reactor d = defer.Deferred() reactor.callLater(0.1, d.callback, result) return d diff --git a/scrapy/utils/ossignal.py b/scrapy/utils/ossignal.py index 7a7aec9be..45c9cef0c 100644 --- a/scrapy/utils/ossignal.py +++ b/scrapy/utils/ossignal.py @@ -1,7 +1,5 @@ import signal -from twisted.internet import reactor - signal_names = {} for signame in dir(signal): @@ -17,6 +15,7 @@ def install_shutdown_handlers(function, override_sigint=True): SIGINT handler won't be install if there is already a handler in place (e.g. Pdb) """ + from twisted.internet import reactor reactor._handleSignals() signal.signal(signal.SIGTERM, function) if signal.getsignal(signal.SIGINT) == signal.default_int_handler or \ diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py index 493d26d4c..b98fff6ec 100644 --- a/scrapy/utils/reactor.py +++ b/scrapy/utils/reactor.py @@ -1,8 +1,9 @@ -from twisted.internet import reactor, error +from twisted.internet import error def listen_tcp(portrange, host, factory): """Like reactor.listenTCP but tries different ports in a range.""" + from twisted.internet import reactor assert len(portrange) <= 2, "invalid portrange: %s" % portrange if not portrange: return reactor.listenTCP(0, factory, interface=host) @@ -30,6 +31,7 @@ class CallLaterOnce(object): self._call = None def schedule(self, delay=0): + from twisted.internet import reactor if self._call is None: self._call = reactor.callLater(delay, self) From 855bbebc8bb862aa02e48f65fd861b1ddf78b57a Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 13 Dec 2019 18:11:49 +0500 Subject: [PATCH 18/32] Move install_asyncio_reactor() from commands to CrawlerProcess. --- scrapy/commands/crawl.py | 3 --- scrapy/commands/runspider.py | 3 --- scrapy/crawler.py | 3 +++ 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index b50761e4a..8093fd402 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -1,6 +1,5 @@ import os from scrapy.commands import ScrapyCommand -from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.conf import arglist_to_dict from scrapy.utils.python import without_none_values from scrapy.exceptions import UsageError @@ -27,8 +26,6 @@ class Command(ScrapyCommand): def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) - if self.settings.getbool('ASYNCIO_ENABLED'): - install_asyncio_reactor() try: opts.spargs = arglist_to_dict(opts.spargs) except ValueError: diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index bfe844eb5..57d8471ca 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -2,7 +2,6 @@ import sys import os from importlib import import_module -from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.spider import iter_spider_classes from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError @@ -51,8 +50,6 @@ class Command(ScrapyCommand): def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) - if self.settings.getbool('ASYNCIO_ENABLED'): - install_asyncio_reactor() try: opts.spargs = arglist_to_dict(opts.spargs) except ValueError: diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 450260004..706c8a59d 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -14,6 +14,7 @@ from scrapy.extension import ExtensionManager from scrapy.settings import overridden_settings, Settings from scrapy.signalmanager import SignalManager from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.asyncio import install_asyncio_reactor from scrapy.utils.ossignal import install_shutdown_handlers, signal_names from scrapy.utils.misc import load_object from scrapy.utils.log import ( @@ -256,6 +257,8 @@ class CrawlerProcess(CrawlerRunner): def __init__(self, settings=None, install_root_handler=True): super(CrawlerProcess, self).__init__(settings) + if self.settings.getbool('ASYNCIO_ENABLED'): + install_asyncio_reactor() install_shutdown_handlers(self._signal_shutdown) configure_logging(self.settings, install_root_handler) log_scrapy_info(self.settings) From bfb78b8dea44a5db3f4a3bca83ab58c7ca0e3ef3 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 13 Dec 2019 18:12:07 +0500 Subject: [PATCH 19/32] Add CrawlerProcess tests for ASYNCIO_ENABLED. --- .../asyncio_enabled_no_reactor.py | 17 ++++++++++++++ .../CrawlerProcess/asyncio_enabled_reactor.py | 22 +++++++++++++++++++ tests/test_crawler.py | 11 ++++++++++ 3 files changed, 50 insertions(+) create mode 100644 tests/CrawlerProcess/asyncio_enabled_no_reactor.py create mode 100644 tests/CrawlerProcess/asyncio_enabled_reactor.py diff --git a/tests/CrawlerProcess/asyncio_enabled_no_reactor.py b/tests/CrawlerProcess/asyncio_enabled_no_reactor.py new file mode 100644 index 000000000..dfe028ef4 --- /dev/null +++ b/tests/CrawlerProcess/asyncio_enabled_no_reactor.py @@ -0,0 +1,17 @@ +import scrapy +from scrapy.crawler import CrawlerProcess + + +class NoRequestsSpider(scrapy.Spider): + name = 'no_request' + + def start_requests(self): + return [] + + +process = CrawlerProcess(settings={ + 'ASYNCIO_ENABLED': True, +}) + +process.crawl(NoRequestsSpider) +process.start() diff --git a/tests/CrawlerProcess/asyncio_enabled_reactor.py b/tests/CrawlerProcess/asyncio_enabled_reactor.py new file mode 100644 index 000000000..7a172ea28 --- /dev/null +++ b/tests/CrawlerProcess/asyncio_enabled_reactor.py @@ -0,0 +1,22 @@ +import asyncio + +from twisted.internet import asyncioreactor +asyncioreactor.install(asyncio.get_event_loop()) + +import scrapy +from scrapy.crawler import CrawlerProcess + + +class NoRequestsSpider(scrapy.Spider): + name = 'no_request' + + def start_requests(self): + return [] + + +process = CrawlerProcess(settings={ + 'ASYNCIO_ENABLED': True, +}) + +process.crawl(NoRequestsSpider) +process.start() diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 05909d995..0b2645280 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -301,3 +301,14 @@ class CrawlerProcessSubprocess(unittest.TestCase): def test_simple(self): log = self.run_script('simple.py') self.assertIn('Spider closed (finished)', log) + self.assertNotIn("DEBUG: Asyncio support enabled", log) + + def test_asyncio_enabled_no_reactor(self): + log = self.run_script('asyncio_enabled_no_reactor.py') + self.assertIn('Spider closed (finished)', log) + self.assertIn("DEBUG: Asyncio support enabled", log) + + def test_asyncio_enabled_reactor(self): + log = self.run_script('asyncio_enabled_reactor.py') + self.assertIn('Spider closed (finished)', log) + self.assertIn("DEBUG: Asyncio support enabled", log) From afc886e57865e82e63f3f8f3326f481908919086 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 13 Dec 2019 19:34:47 +0500 Subject: [PATCH 20/32] Simplify tox.ini asyncio entries. --- tox.ini | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tox.ini b/tox.ini index a4edae439..795c20233 100644 --- a/tox.ini +++ b/tox.ini @@ -107,14 +107,16 @@ deps = reppy robotexclusionrulesparser +[asyncio] +commands = + py.test --cov=scrapy --cov-report= --reactor=asyncio {posargs:scrapy tests} + [testenv:py35-asyncio] basepython = python3.5 deps = {[testenv]deps} -commands = - py.test --cov=scrapy --cov-report= --reactor=asyncio {posargs:scrapy tests} +commands = {[asyncio]commands} [testenv:py38-asyncio] basepython = python3.8 deps = {[testenv]deps} -commands = - py.test --cov=scrapy --cov-report= --reactor=asyncio {posargs:scrapy tests} +commands = {[asyncio]commands} From a1605cade6286dd5f7f1c9e4c9660d44ed15ed19 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 13 Dec 2019 19:35:09 +0500 Subject: [PATCH 21/32] Hide utils.defer.isfuture(). --- scrapy/utils/defer.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 6a91776c7..530bf0e9d 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -2,7 +2,6 @@ Helper functions for dealing with Twisted deferreds """ import asyncio -import asyncio.futures import inspect from twisted.internet import defer, task @@ -121,18 +120,18 @@ def iter_errback(iterable, errback, *a, **kw): errback(failure.Failure(), *a, **kw) -def isfuture(o): +def _isfuture(o): # workaround for Python before 3.5.3 not having asyncio.isfuture if hasattr(asyncio, 'isfuture'): return asyncio.isfuture(o) - return isinstance(o, asyncio.futures.Future) + return isinstance(o, asyncio.Future) def deferred_from_coro(o, asyncio_enabled=False): """Converts a coroutine into a Deferred, or returns the object as is if it isn't a coroutine""" if isinstance(o, defer.Deferred): return o - if isfuture(o) or inspect.isawaitable(o): + if _isfuture(o) or inspect.isawaitable(o): if not asyncio_enabled: # wrapping the coroutine directly into a Deferred, this doesn't work correctly with coroutines # that use asyncio, e.g. "await asyncio.sleep(1)" From 2db7d453788f5c638d0921b0f7f8bab58e2a58bc Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 16 Dec 2019 19:24:25 +0500 Subject: [PATCH 22/32] Enable skipping tests based on --reactor. --- conftest.py | 8 ++++++++ pytest.ini | 2 ++ 2 files changed, 10 insertions(+) diff --git a/conftest.py b/conftest.py index 64136b48d..56d552953 100644 --- a/conftest.py +++ b/conftest.py @@ -35,6 +35,14 @@ def pytest_collection_modifyitems(session, config, items): except ImportError: pass + @pytest.fixture() def reactor_pytest(request): request.cls.reactor_pytest = request.config.getoption("--reactor") + return request.cls.reactor_pytest + + +@pytest.fixture(autouse=True) +def only_asyncio(request, reactor_pytest): + if request.node.get_closest_marker('only_asyncio') and reactor_pytest != 'asyncio': + pytest.skip('This test is only run with --reactor-asyncio') diff --git a/pytest.ini b/pytest.ini index 336ef041d..7b62a1bd8 100644 --- a/pytest.ini +++ b/pytest.ini @@ -19,6 +19,8 @@ addopts = --ignore=docs/topics/telnetconsole.rst --ignore=docs/utils twisted = 1 +markers = + only_asyncio: marks tests as only enabled when --reactor=asyncio is passed flake8-ignore = # Files that are only meant to provide top-level imports are expected not # to use any of their imports: From 039e6fe6919341dbfd864c4a406d35389c8e2992 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 16 Dec 2019 20:17:41 +0500 Subject: [PATCH 23/32] Refactor install_asyncio_reactor slightly. --- scrapy/utils/asyncio.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scrapy/utils/asyncio.py b/scrapy/utils/asyncio.py index b5d5f92d9..b53c8a8b0 100644 --- a/scrapy/utils/asyncio.py +++ b/scrapy/utils/asyncio.py @@ -1,3 +1,8 @@ +from contextlib import suppress + +from twisted.internet.error import ReactorAlreadyInstalledError + + def install_asyncio_reactor(): """ Tries to install AsyncioSelectorReactor """ @@ -5,13 +10,10 @@ def install_asyncio_reactor(): import asyncio from twisted.internet import asyncioreactor except ImportError: - pass - else: - from twisted.internet.error import ReactorAlreadyInstalledError - try: - asyncioreactor.install(asyncio.get_event_loop()) - except ReactorAlreadyInstalledError: - pass + return + + with suppress(ReactorAlreadyInstalledError): + asyncioreactor.install(asyncio.get_event_loop()) def is_asyncio_reactor_installed(): From 900de7c14607fbe2936fa682d03747916337f075 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 16 Dec 2019 21:11:58 +0500 Subject: [PATCH 24/32] Fix the reactor_pytest fixture. --- conftest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 56d552953..6d9696a3f 100644 --- a/conftest.py +++ b/conftest.py @@ -36,8 +36,11 @@ def pytest_collection_modifyitems(session, config, items): pass -@pytest.fixture() +@pytest.fixture(scope='class') def reactor_pytest(request): + if not request.cls: + # doctests + return request.cls.reactor_pytest = request.config.getoption("--reactor") return request.cls.reactor_pytest From 40697dcbfa17dccf81adce7d033bc466ba6e98a2 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 20 Dec 2019 19:33:44 +0500 Subject: [PATCH 25/32] Remove deferred_from_coro from this PR. --- scrapy/utils/defer.py | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 530bf0e9d..20ce59297 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -1,14 +1,10 @@ """ Helper functions for dealing with Twisted deferreds """ -import asyncio -import inspect - from twisted.internet import defer, task from twisted.python import failure from scrapy.exceptions import IgnoreRequest -from scrapy.utils.asyncio import is_asyncio_reactor_installed def defer_fail(_failure): @@ -118,27 +114,3 @@ def iter_errback(iterable, errback, *a, **kw): break except Exception: errback(failure.Failure(), *a, **kw) - - -def _isfuture(o): - # workaround for Python before 3.5.3 not having asyncio.isfuture - if hasattr(asyncio, 'isfuture'): - return asyncio.isfuture(o) - return isinstance(o, asyncio.Future) - - -def deferred_from_coro(o, asyncio_enabled=False): - """Converts a coroutine into a Deferred, or returns the object as is if it isn't a coroutine""" - if isinstance(o, defer.Deferred): - return o - if _isfuture(o) or inspect.isawaitable(o): - if not asyncio_enabled: - # wrapping the coroutine directly into a Deferred, this doesn't work correctly with coroutines - # that use asyncio, e.g. "await asyncio.sleep(1)" - return defer.ensureDeferred(o) - else: - # wrapping the coroutine into a Future and then into a Deferred, this requires AsyncioSelectorReactor - if not is_asyncio_reactor_installed(): - raise TypeError('Using coroutines requires installing AsyncioSelectorReactor') - return defer.Deferred.fromFuture(asyncio.ensure_future(o)) - return o From e342de5038e3757660f947fe1fadf35b54cd7113 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 20 Dec 2019 19:37:50 +0500 Subject: [PATCH 26/32] Remove a stray newline. --- tests/mockserver.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/mockserver.py b/tests/mockserver.py index d4e0362fb..a45277db9 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -6,7 +6,6 @@ from subprocess import Popen, PIPE from urllib.parse import urlencode from OpenSSL import SSL - from twisted.web.server import Site, NOT_DONE_YET from twisted.web.resource import Resource from twisted.web.static import File From 8de80f59db19d739a056a7e58662f90544fece16 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Sat, 21 Dec 2019 13:08:29 +0500 Subject: [PATCH 27/32] Raise an exception if ASYNCIO_ENABLED but the reactor is wrong. --- scrapy/crawler.py | 6 +++++- scrapy/utils/log.py | 8 +------- tests/test_crawler.py | 9 +++++---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 706c8a59d..a9443f7ac 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -14,7 +14,7 @@ from scrapy.extension import ExtensionManager from scrapy.settings import overridden_settings, Settings from scrapy.signalmanager import SignalManager from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.asyncio import install_asyncio_reactor +from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_reactor_installed from scrapy.utils.ossignal import install_shutdown_handlers, signal_names from scrapy.utils.misc import load_object from scrapy.utils.log import ( @@ -259,6 +259,10 @@ class CrawlerProcess(CrawlerRunner): super(CrawlerProcess, self).__init__(settings) if self.settings.getbool('ASYNCIO_ENABLED'): install_asyncio_reactor() + if not is_asyncio_reactor_installed(): + raise Exception("ASYNCIO_ENABLED is on but the Twisted asyncio " + "reactor is not installed, this is not supported.") + install_shutdown_handlers(self._signal_shutdown) configure_logging(self.settings, install_root_handler) log_scrapy_info(self.settings) diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 0fe3d1549..6179e1bd1 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -11,7 +11,6 @@ from twisted.python import log as twisted_log import scrapy from scrapy.settings import Settings from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.asyncio import is_asyncio_reactor_installed from scrapy.utils.versions import scrapy_components_versions @@ -150,12 +149,7 @@ def log_scrapy_info(settings): for name, version in scrapy_components_versions() if name != "Scrapy")}) if settings.getbool('ASYNCIO_ENABLED'): - if is_asyncio_reactor_installed(): - logger.debug("Asyncio support enabled") - else: - logger.error("ASYNCIO_ENABLED is on but the Twisted asyncio " - "reactor is not installed, this is not supported " - "and asyncio coroutines will not work.") + logger.debug("Asyncio support enabled") class StreamLogger(object): diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 0b2645280..a2865fcd1 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -264,13 +264,14 @@ class CrawlerRunnerHasSpider(unittest.TestCase): @defer.inlineCallbacks def test_crawler_process_asyncio_enabled_true(self): with LogCapture(level=logging.DEBUG) as log: - runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': True}) - yield runner.crawl(NoRequestsSpider) if self.reactor_pytest == 'asyncio': + runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': True}) + yield runner.crawl(NoRequestsSpider) self.assertIn("Asyncio support enabled", str(log)) else: - self.assertNotIn("Asyncio support enabled", str(log)) - self.assertIn("ASYNCIO_ENABLED is on but the Twisted asyncio reactor is not installed", str(log)) + msg = "ASYNCIO_ENABLED is on but the Twisted asyncio reactor is not installed" + with self.assertRaisesRegex(Exception, msg): + runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': True}) @defer.inlineCallbacks def test_crawler_process_asyncio_enabled_false(self): From 87ece066ca320b07acda57c99aee8a62992ec144 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 26 Dec 2019 20:41:06 +0500 Subject: [PATCH 28/32] Remove conditional asyncio imports. --- scrapy/utils/asyncio.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/scrapy/utils/asyncio.py b/scrapy/utils/asyncio.py index b53c8a8b0..917973de2 100644 --- a/scrapy/utils/asyncio.py +++ b/scrapy/utils/asyncio.py @@ -1,25 +1,17 @@ +import asyncio from contextlib import suppress +from twisted.internet import asyncioreactor from twisted.internet.error import ReactorAlreadyInstalledError def install_asyncio_reactor(): """ Tries to install AsyncioSelectorReactor """ - try: - import asyncio - from twisted.internet import asyncioreactor - except ImportError: - return - with suppress(ReactorAlreadyInstalledError): asyncioreactor.install(asyncio.get_event_loop()) def is_asyncio_reactor_installed(): - try: - import twisted.internet.reactor - from twisted.internet import asyncioreactor - return isinstance(twisted.internet.reactor, asyncioreactor.AsyncioSelectorReactor) - except ImportError: - return False + from twisted.internet import reactor + return isinstance(reactor, asyncioreactor.AsyncioSelectorReactor) From 37ac47ff8074959ea66566fcb8b0e9e62272f963 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 26 Dec 2019 20:46:54 +0500 Subject: [PATCH 29/32] Fix a deprecation warning. --- tests/test_utils_asyncio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index a6ba24876..44acc24af 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -10,7 +10,7 @@ class AsyncioTest(TestCase): def test_is_asyncio_reactor_installed(self): # the result should depend only on the pytest --reactor argument - self.assertEquals(is_asyncio_reactor_installed(), self.reactor_pytest == 'asyncio') + self.assertEqual(is_asyncio_reactor_installed(), self.reactor_pytest == 'asyncio') def test_install_asyncio_reactor(self): # this should do nothing From 30ebd05a5f627262702ad1e1e488d6176a8c7882 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 27 Dec 2019 00:05:14 +0500 Subject: [PATCH 30/32] Simplify the tox asyncio entries. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ed0d4c9ab..b62100026 100644 --- a/tox.ini +++ b/tox.ini @@ -99,7 +99,7 @@ commands = [asyncio] commands = - py.test --cov=scrapy --cov-report= --reactor=asyncio {posargs:scrapy tests} + {[testenv]commands} --reactor=asyncio [testenv:py35-asyncio] basepython = python3.5 From f75ccc997aa75fadf96a8d4b836248397ef89802 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 27 Dec 2019 19:48:54 +0500 Subject: [PATCH 31/32] FIx a typo in the only_asyncio fixture. --- conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 6d9696a3f..c0de09909 100644 --- a/conftest.py +++ b/conftest.py @@ -48,4 +48,4 @@ def reactor_pytest(request): @pytest.fixture(autouse=True) def only_asyncio(request, reactor_pytest): if request.node.get_closest_marker('only_asyncio') and reactor_pytest != 'asyncio': - pytest.skip('This test is only run with --reactor-asyncio') + pytest.skip('This test is only run with --reactor=asyncio') From dc1ee09481c7655a8ebf77a75ccc965a4ba5400d Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 27 Dec 2019 21:55:58 +0500 Subject: [PATCH 32/32] Rename ASYNCIO_ENABLED to ASYNCIO_REACTOR, change the logic accordingly. --- docs/topics/settings.rst | 14 +++---- scrapy/crawler.py | 17 +++++--- scrapy/settings/default_settings.py | 2 +- scrapy/utils/log.py | 5 ++- .../asyncio_enabled_no_reactor.py | 2 +- .../CrawlerProcess/asyncio_enabled_reactor.py | 2 +- tests/test_commands.py | 8 ++-- tests/test_crawler.py | 42 ++++++++----------- 8 files changed, 43 insertions(+), 49 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 62b2870b2..c02f877fc 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -160,26 +160,22 @@ to any particular component. In that case the module of that component will be shown, typically an extension, middleware or pipeline. It also means that the component must be enabled in order for the setting to have any effect. -.. setting:: ASYNCIO_ENABLED +.. setting:: ASYNCIO_REACTOR -ASYNCIO_ENABLED +ASYNCIO_REACTOR --------------- Default: ``False`` -Whether to support ``async def`` methods and callbacks which use code that -requires an asyncio loop. - -If an ``async def`` coroutine doesn't require the asyncio loop, it will work -even if this is set to ``False``. Coroutines that require the asyncio loop may -silently fail to run or raise errors unless this is set to ``True``. +Whether to install and require the Twisted reactor that uses the asyncio loop. When this option is set to ``True``, Scrapy will require :class:`~twisted.internet.asyncioreactor.AsyncioSelectorReactor`. It will install this reactor if no reactor is installed yet, such as when using the ``scrapy`` script or :class:`~scrapy.crawler.CrawlerProcess`. If you are using :class:`~scrapy.crawler.CrawlerRunner`, you need to install the correct reactor -manually. +manually. If a different reactor is installed outside Scrapy, it will raise an +exception. The default value for this option is currently ``False`` to maintain backward compatibility and avoid possible problems caused by using a different Twisted diff --git a/scrapy/crawler.py b/scrapy/crawler.py index a9443f7ac..f87e67d93 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -137,6 +137,7 @@ class CrawlerRunner(object): self._crawlers = set() self._active = set() self.bootstrap_failed = False + self._handle_asyncio_reactor() @property def spiders(self): @@ -230,6 +231,11 @@ class CrawlerRunner(object): while self._active: yield defer.DeferredList(self._active) + def _handle_asyncio_reactor(self): + if self.settings.getbool('ASYNCIO_REACTOR') and not is_asyncio_reactor_installed(): + raise Exception("ASYNCIO_REACTOR is on but the Twisted asyncio " + "reactor is not installed.") + class CrawlerProcess(CrawlerRunner): """ @@ -257,12 +263,6 @@ class CrawlerProcess(CrawlerRunner): def __init__(self, settings=None, install_root_handler=True): super(CrawlerProcess, self).__init__(settings) - if self.settings.getbool('ASYNCIO_ENABLED'): - install_asyncio_reactor() - if not is_asyncio_reactor_installed(): - raise Exception("ASYNCIO_ENABLED is on but the Twisted asyncio " - "reactor is not installed, this is not supported.") - install_shutdown_handlers(self._signal_shutdown) configure_logging(self.settings, install_root_handler) log_scrapy_info(self.settings) @@ -333,6 +333,11 @@ class CrawlerProcess(CrawlerRunner): except RuntimeError: # raised if already stopped or in shutdown stage pass + def _handle_asyncio_reactor(self): + if self.settings.getbool('ASYNCIO_REACTOR'): + install_asyncio_reactor() + super()._handle_asyncio_reactor() + def _get_spider_loader(settings): """ Get SpiderLoader instance from settings """ diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index a7792e248..d03fd37b0 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -19,7 +19,7 @@ from os.path import join, abspath, dirname AJAXCRAWL_ENABLED = False -ASYNCIO_ENABLED = False +ASYNCIO_REACTOR = False AUTOTHROTTLE_ENABLED = False AUTOTHROTTLE_DEBUG = False diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 6179e1bd1..e4cf0196b 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -11,6 +11,7 @@ from twisted.python import log as twisted_log import scrapy from scrapy.settings import Settings from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.asyncio import is_asyncio_reactor_installed from scrapy.utils.versions import scrapy_components_versions @@ -148,8 +149,8 @@ def log_scrapy_info(settings): {'versions': ", ".join("%s %s" % (name, version) for name, version in scrapy_components_versions() if name != "Scrapy")}) - if settings.getbool('ASYNCIO_ENABLED'): - logger.debug("Asyncio support enabled") + if is_asyncio_reactor_installed(): + logger.debug("Asyncio reactor is installed") class StreamLogger(object): diff --git a/tests/CrawlerProcess/asyncio_enabled_no_reactor.py b/tests/CrawlerProcess/asyncio_enabled_no_reactor.py index dfe028ef4..db1b75931 100644 --- a/tests/CrawlerProcess/asyncio_enabled_no_reactor.py +++ b/tests/CrawlerProcess/asyncio_enabled_no_reactor.py @@ -10,7 +10,7 @@ class NoRequestsSpider(scrapy.Spider): process = CrawlerProcess(settings={ - 'ASYNCIO_ENABLED': True, + 'ASYNCIO_REACTOR': True, }) process.crawl(NoRequestsSpider) diff --git a/tests/CrawlerProcess/asyncio_enabled_reactor.py b/tests/CrawlerProcess/asyncio_enabled_reactor.py index 7a172ea28..cec3c9c25 100644 --- a/tests/CrawlerProcess/asyncio_enabled_reactor.py +++ b/tests/CrawlerProcess/asyncio_enabled_reactor.py @@ -15,7 +15,7 @@ class NoRequestsSpider(scrapy.Spider): process = CrawlerProcess(settings={ - 'ASYNCIO_ENABLED': True, + 'ASYNCIO_REACTOR': True, }) process.crawl(NoRequestsSpider) diff --git a/tests/test_commands.py b/tests/test_commands.py index 197d80217..6024af71c 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -296,12 +296,12 @@ class BadSpider(scrapy.Spider): self.assertIn("badspider.py", log) def test_asyncio_enabled_true(self): - log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_ENABLED=True']) - self.assertIn("DEBUG: Asyncio support enabled", log) + log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_REACTOR=True']) + self.assertIn("DEBUG: Asyncio reactor is installed", log) def test_asyncio_enabled_false(self): - log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_ENABLED=False']) - self.assertNotIn("DEBUG: Asyncio support enabled", log) + log = self.get_log(self.debug_log_spider, args=['-s', 'ASYNCIO_REACTOR=False']) + self.assertNotIn("DEBUG: Asyncio reactor is installed", log) class BenchCommandTest(CommandTest): diff --git a/tests/test_crawler.py b/tests/test_crawler.py index a2865fcd1..fce60ca37 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -13,7 +13,6 @@ import scrapy from scrapy.crawler import Crawler, CrawlerRunner, CrawlerProcess from scrapy.settings import Settings, default_settings from scrapy.spiderloader import SpiderLoader -from scrapy.utils.asyncio import is_asyncio_reactor_installed from scrapy.utils.log import configure_logging, get_scrapy_root_handler from scrapy.utils.spider import DefaultSpider from scrapy.utils.misc import load_object @@ -209,14 +208,6 @@ class NoRequestsSpider(scrapy.Spider): return [] -class AsyncioSpider(scrapy.Spider): - name = 'asyncio' - - def start_requests(self): - self.logger.info('Asyncio support: %s', is_asyncio_reactor_installed()) - return [] - - @mark.usefixtures('reactor_pytest') class CrawlerRunnerHasSpider(unittest.TestCase): @@ -261,31 +252,32 @@ class CrawlerRunnerHasSpider(unittest.TestCase): self.assertEqual(runner.bootstrap_failed, True) + def test_crawler_runner_asyncio_enabled_true(self): + if self.reactor_pytest == 'asyncio': + runner = CrawlerRunner(settings={'ASYNCIO_REACTOR': True}) + else: + msg = "ASYNCIO_REACTOR is on but the Twisted asyncio reactor is not installed" + with self.assertRaisesRegex(Exception, msg): + runner = CrawlerRunner(settings={'ASYNCIO_REACTOR': True}) + @defer.inlineCallbacks def test_crawler_process_asyncio_enabled_true(self): with LogCapture(level=logging.DEBUG) as log: if self.reactor_pytest == 'asyncio': - runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': True}) + runner = CrawlerProcess(settings={'ASYNCIO_REACTOR': True}) yield runner.crawl(NoRequestsSpider) - self.assertIn("Asyncio support enabled", str(log)) + self.assertIn("Asyncio reactor is installed", str(log)) else: - msg = "ASYNCIO_ENABLED is on but the Twisted asyncio reactor is not installed" + msg = "ASYNCIO_REACTOR is on but the Twisted asyncio reactor is not installed" with self.assertRaisesRegex(Exception, msg): - runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': True}) + runner = CrawlerProcess(settings={'ASYNCIO_REACTOR': True}) @defer.inlineCallbacks def test_crawler_process_asyncio_enabled_false(self): - runner = CrawlerProcess(settings={'ASYNCIO_ENABLED': False}) + runner = CrawlerProcess(settings={'ASYNCIO_REACTOR': False}) with LogCapture(level=logging.DEBUG) as log: yield runner.crawl(NoRequestsSpider) - self.assertNotIn("Asyncio support enabled", str(log)) - - @defer.inlineCallbacks - def test_crawler_runner_asyncio_supported(self): - runner = CrawlerRunner() - with LogCapture() as log: - yield runner.crawl(AsyncioSpider) - log.check_present(('asyncio', 'INFO', 'Asyncio support: %s' % (self.reactor_pytest == 'asyncio'))) + self.assertNotIn("Asyncio reactor is installed", str(log)) class CrawlerProcessSubprocess(unittest.TestCase): @@ -302,14 +294,14 @@ class CrawlerProcessSubprocess(unittest.TestCase): def test_simple(self): log = self.run_script('simple.py') self.assertIn('Spider closed (finished)', log) - self.assertNotIn("DEBUG: Asyncio support enabled", log) + self.assertNotIn("DEBUG: Asyncio reactor is installed", log) def test_asyncio_enabled_no_reactor(self): log = self.run_script('asyncio_enabled_no_reactor.py') self.assertIn('Spider closed (finished)', log) - self.assertIn("DEBUG: Asyncio support enabled", log) + self.assertIn("DEBUG: Asyncio reactor is installed", log) def test_asyncio_enabled_reactor(self): log = self.run_script('asyncio_enabled_reactor.py') self.assertIn('Spider closed (finished)', log) - self.assertIn("DEBUG: Asyncio support enabled", log) + self.assertIn("DEBUG: Asyncio reactor is installed", log)