From c058156c4884bec5b8efbf6a4bac54786adb7746 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 8 May 2020 18:33:56 +0500 Subject: [PATCH] Better idea for universal process_start_requests(). --- docs/topics/coroutines.rst | 34 ++++++++++++--- scrapy/utils/asyncgen.py | 11 +++++ scrapy/utils/middlewares.py | 25 +++++++++++ tests/test_spidermiddleware.py | 78 +++++++++++++++++++++++++++++++--- 4 files changed, 135 insertions(+), 13 deletions(-) create mode 100644 scrapy/utils/middlewares.py diff --git a/docs/topics/coroutines.rst b/docs/topics/coroutines.rst index 3b9d9e272..d4f9af86b 100644 --- a/docs/topics/coroutines.rst +++ b/docs/topics/coroutines.rst @@ -65,16 +65,19 @@ spider that have the :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_start_requests` method must support this: if they receive an asynchronous iterable, they must return one as well. On the other hand, if they receive a normal iterable, they -shouldn't break. Such universal :meth:`process_start_requests` must be an -asynchronous generator itself, and so it will always convert a normal iterable -to an asynchronous one. Because a result of a middleware method is passed to -the same method of the next middleware, it's only possible to mix middlewares -with synchronous and asynchronous :meth:`process_start_requests` if all -synchronous ones are called first. +shouldn't break and ideally should return a normal iterable too. There can be +several possible implementations of this. + +First, such universal :meth:`process_start_requests` can be an asynchronous +generator itself, and so it will always convert a normal iterable to an +asynchronous one. Because a result of a middleware method is passed to the same +method of the next middleware, it's only possible to mix middlewares with +synchronous and asynchronous :meth:`process_start_requests` if all synchronous +ones are called first. .. autofunction:: scrapy.utils.asyncgen.as_async_generator -Here is an example of a universal middleware:: +Here is an example of a universal middleware using this approach:: from scrapy.utils.asyncgen import as_async_generator @@ -87,6 +90,23 @@ Here is an example of a universal middleware:: If this method includes asynchronous code, that code will work even with synchronous :meth:`~scrapy.spiders.Spider.start_requests`. +Another option is to make separate methods for normal and asynchronous +iterables and choose one at the run time:: + + class ProcessStartRequestsAsyncGenMiddleware: + def _normal_process_start_requests(self, start_requests, spider): + # ... do something with normal start_requests + + async def _async_process_start_requests(self, start_requests, spider): + # ... do something with async start_requests + + def process_start_requests(self, start_requests, spider): + if hasattr(inspect, 'isasyncgen') and inspect.isasyncgen(start_requests): + return self._async_process_start_requests(start_requests, spider) + else: + return self._normal_process_start_requests(start_requests, spider) + + Usage ===== diff --git a/scrapy/utils/asyncgen.py b/scrapy/utils/asyncgen.py index 6130e81be..a076d840e 100644 --- a/scrapy/utils/asyncgen.py +++ b/scrapy/utils/asyncgen.py @@ -18,3 +18,14 @@ async def as_async_generator(it): else: for r in it: yield r + + +async def process_async_iterable_helper(it, in_predicate=None, out_predicate=None, processor=None): + async for o in it: + if in_predicate and not in_predicate(o): + continue + if processor is not None: + o = processor(o) + if out_predicate and not out_predicate(o): + return + yield o diff --git a/scrapy/utils/middlewares.py b/scrapy/utils/middlewares.py new file mode 100644 index 000000000..e60cea8f9 --- /dev/null +++ b/scrapy/utils/middlewares.py @@ -0,0 +1,25 @@ +# coding: utf-8 +import inspect + + +def process_normal_iterable_helper(it, in_predicate=None, out_predicate=None, processor=None): + for o in it: + if in_predicate and not in_predicate(o): + continue + if processor is not None: + o = processor(o) + if out_predicate and not out_predicate(o): + continue + yield o + + +def process_iterable_helper(it, in_predicate=None, out_predicate=None, processor=None): + """ + For each item in the iterable: skips it if in_predicate is False, applies processor, + skips the result if out_predicate is False, else yields it. + """ + if hasattr(inspect, 'isasyncgen') and inspect.isasyncgen(it): + from scrapy.utils.asyncgen import process_async_iterable_helper # Python 3.5 limitation + return process_async_iterable_helper(it, in_predicate, out_predicate, processor) + else: + return process_normal_iterable_helper(it, in_predicate, out_predicate, processor) diff --git a/tests/test_spidermiddleware.py b/tests/test_spidermiddleware.py index ce84ae187..44c35b506 100644 --- a/tests/test_spidermiddleware.py +++ b/tests/test_spidermiddleware.py @@ -11,6 +11,7 @@ from scrapy.spiders import Spider from scrapy.http import Request, Response from scrapy.exceptions import _InvalidOutput from scrapy.utils.defer import deferred_from_coro +from scrapy.utils.middlewares import process_iterable_helper from scrapy.utils.test import get_crawler from scrapy.core.spidermw import SpiderMiddlewareManager from tests.test_engine import StartRequestsAsyncDefSpider @@ -115,6 +116,11 @@ class ProcessStartRequestsSimpleMiddleware: yield r +class ProcessStartRequestsUniversalMiddleware: + def process_start_requests(self, start_requests, spider): + return process_iterable_helper(start_requests) + + class ProcessStartRequestsSimple(TestCase): """ process_start_requests tests for simple start_requests""" @@ -137,9 +143,8 @@ class ProcessStartRequestsSimple(TestCase): list(processed_start_requests) @defer.inlineCallbacks - def test_simple(self): - """ Simple mw """ - processed_start_requests = yield self._get_processed_start_requests(ProcessStartRequestsSimpleMiddleware) + def _test_simple_base(self, *mw_classes): + processed_start_requests = yield self._get_processed_start_requests(*mw_classes) self.assertTrue(inspect.isgenerator(processed_start_requests)) start_requests_list = list(processed_start_requests) self.assertEqual(len(start_requests_list), 3) @@ -154,6 +159,11 @@ class ProcessStartRequestsSimple(TestCase): self.assertEqual(len(start_requests_list), 3) self.assertIsInstance(start_requests_list[0], Request) + @defer.inlineCallbacks + def test_simple(self): + """ Simple mw """ + yield self._test_simple_base(ProcessStartRequestsSimpleMiddleware) + @mark.skipif(sys.version_info < (3, 6), reason="Async generators require Python 3.6 or higher") @defer.inlineCallbacks def test_asyncgen(self): @@ -163,7 +173,7 @@ class ProcessStartRequestsSimple(TestCase): @mark.skipif(sys.version_info < (3, 6), reason="Async generators require Python 3.6 or higher") @defer.inlineCallbacks - def test_asyncgen2(self): + def test_simple_asyncgen(self): """ Simple mw -> asyncgen mw """ from tests.py36._test_spidermiddleware import ProcessStartRequestsAsyncGenMiddleware yield self._test_asyncgen_base(ProcessStartRequestsAsyncGenMiddleware, @@ -171,7 +181,7 @@ class ProcessStartRequestsSimple(TestCase): @mark.skipif(sys.version_info < (3, 6), reason="Async generators require Python 3.6 or higher") @defer.inlineCallbacks - def test_asyncgen3(self): + def test_asyncgen_simple(self): """ Asyncgen mw -> simple mw; cannot work """ from tests.py36._test_spidermiddleware import ProcessStartRequestsAsyncGenMiddleware processed_start_requests = yield self._get_processed_start_requests( @@ -180,6 +190,39 @@ class ProcessStartRequestsSimple(TestCase): self.assertTrue(inspect.isgenerator(processed_start_requests)) self.assertAsyncGeneratorNotIterable(processed_start_requests) + @defer.inlineCallbacks + def test_universal(self): + """ Universal mw """ + yield self._test_simple_base(ProcessStartRequestsUniversalMiddleware) + + @defer.inlineCallbacks + def test_universal_simple(self): + """ Universal mw -> simple mw """ + yield self._test_simple_base(ProcessStartRequestsSimpleMiddleware, + ProcessStartRequestsUniversalMiddleware) + + @defer.inlineCallbacks + def test_simple_universal(self): + """ Simple mw -> universal mw """ + yield self._test_simple_base(ProcessStartRequestsUniversalMiddleware, + ProcessStartRequestsSimpleMiddleware) + + @mark.skipif(sys.version_info < (3, 6), reason="Async generators require Python 3.6 or higher") + @defer.inlineCallbacks + def test_universal_asyncgen(self): + """ Universal mw -> asyncgen mw """ + from tests.py36._test_spidermiddleware import ProcessStartRequestsAsyncGenMiddleware + yield self._test_asyncgen_base(ProcessStartRequestsAsyncGenMiddleware, + ProcessStartRequestsUniversalMiddleware) + + @mark.skipif(sys.version_info < (3, 6), reason="Async generators require Python 3.6 or higher") + @defer.inlineCallbacks + def test_asyncgen_universal(self): + """ Asyncgen mw -> universal mw """ + from tests.py36._test_spidermiddleware import ProcessStartRequestsAsyncGenMiddleware + yield self._test_asyncgen_base(ProcessStartRequestsUniversalMiddleware, + ProcessStartRequestsAsyncGenMiddleware) + class ProcessStartRequestsAsyncDef(ProcessStartRequestsSimple): """ process_start_requests tests for async def start_requests """ @@ -205,7 +248,7 @@ class ProcessStartRequestsAsyncGen(ProcessStartRequestsSimple): self.assertAsyncGeneratorNotIterable(processed_start_requests) @defer.inlineCallbacks - def test_asyncgen2(self): + def test_simple_asyncgen(self): """ Simple mw -> asyncgen mw; cannot work """ from tests.py36._test_spidermiddleware import ProcessStartRequestsAsyncGenMiddleware processed_start_requests = yield self._get_processed_start_requests( @@ -213,3 +256,26 @@ class ProcessStartRequestsAsyncGen(ProcessStartRequestsSimple): ProcessStartRequestsSimpleMiddleware) self.assertTrue(inspect.isasyncgen(processed_start_requests)) self.assertAsyncGeneratorNotIterable(processed_start_requests) + + @defer.inlineCallbacks + def test_universal(self): + """ Universal mw """ + yield self._test_asyncgen_base(ProcessStartRequestsUniversalMiddleware) + + @defer.inlineCallbacks + def test_universal_simple(self): + """ Universal mw -> simple mw; cannot work """ + processed_start_requests = yield self._get_processed_start_requests( + ProcessStartRequestsSimpleMiddleware, + ProcessStartRequestsUniversalMiddleware) + self.assertTrue(inspect.isgenerator(processed_start_requests)) + self.assertAsyncGeneratorNotIterable(processed_start_requests) + + @defer.inlineCallbacks + def test_simple_universal(self): + """ Simple mw -> universal mw; cannot work """ + processed_start_requests = yield self._get_processed_start_requests( + ProcessStartRequestsUniversalMiddleware, + ProcessStartRequestsSimpleMiddleware) + self.assertTrue(inspect.isgenerator(processed_start_requests)) + self.assertAsyncGeneratorNotIterable(processed_start_requests)