From ed34ce14c0c06d4539d4fdeb0ad014f4e6fb5b94 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 4 Dec 2019 21:32:16 +0500 Subject: [PATCH] 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