diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index ce0e1a6c2..d740808cc 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -33,3 +33,7 @@ jobs: - name: Upload coverage report uses: codecov/codecov-action@v5 + + - name: Upload test results + if: ${{ !cancelled() }} + uses: codecov/test-results-action@v1 diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 444aa3557..34819f227 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -88,3 +88,7 @@ jobs: - name: Upload coverage report uses: codecov/codecov-action@v5 + + - name: Upload test results + if: ${{ !cancelled() }} + uses: codecov/test-results-action@v1 diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml index 537a01e29..bbbb704e5 100644 --- a/.github/workflows/tests-windows.yml +++ b/.github/workflows/tests-windows.yml @@ -64,3 +64,7 @@ jobs: - name: Upload coverage report uses: codecov/codecov-action@v5 + + - name: Upload test results + if: ${{ !cancelled() }} + uses: codecov/test-results-action@v1 diff --git a/.gitignore b/.gitignore index 6c5c50e08..0a3f0ac1c 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ htmlcov/ .pytest_cache/ .coverage.* coverage.* +*.junit.xml test-output.* .cache/ .mypy_cache/ diff --git a/docs/conf.py b/docs/conf.py index a7a9bd46a..1447b253f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -26,7 +26,6 @@ author = "Scrapy developers" # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration extensions = [ - "enum_tools.autoenum", "hoverxref.extension", "notfound.extension", "scrapydocs", diff --git a/docs/contributing.rst b/docs/contributing.rst index bb197b428..f5c1c74b8 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -228,9 +228,10 @@ with a name of the branch you want to create locally). See also: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally. When writing GitHub pull requests, try to keep titles short but descriptive. -E.g. For bug #411: "Scrapy hangs if an exception raises in yield_seeds" prefer -"Fix hanging when exception occurs in yield_seeds (#411)" instead of "Fix for -#411". Complete titles make it easy to skim through the issue tracker. +E.g. For bug #411: "Scrapy hangs if an exception raises in start_requests" +prefer "Fix hanging when exception occurs in start_requests (#411)" +instead of "Fix for #411". Complete titles make it easy to skim through +the issue tracker. Finally, try to keep aesthetic changes (:pep:`8` compliance, unused imports removal, etc) in separate commits from functional changes. This will make pull diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index 8f74f76af..c4e04364b 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -94,7 +94,7 @@ This is the code for our first Spider. Save it in a file named class QuotesSpider(scrapy.Spider): name = "quotes" - async def yield_seeds(self): + async def start(self): urls = [ "https://quotes.toscrape.com/page/1/", "https://quotes.toscrape.com/page/2/", @@ -116,7 +116,7 @@ and defines some attributes and methods: unique within a project, that is, you can't set the same name for different Spiders. -* :meth:`~scrapy.Spider.yield_seeds`: must be an asynchronous generator that +* :meth:`~scrapy.Spider.start`: must be an asynchronous generator that yields requests (and, optionally, items) for the spider to start crawling. Subsequent requests will be generated successively from these initial requests. @@ -165,20 +165,20 @@ What just happened under the hood? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Scrapy sends the first :class:`scrapy.Request ` objects yielded -by the :meth:`~scrapy.Spider.yield_seeds` spider method. Upon receiving a +by the :meth:`~scrapy.Spider.start` spider method. Upon receiving a response for each one, Scrapy calls the callback method associated with the request (in this case, the ``parse`` method) with a :class:`~scrapy.http.Response` object. -A shortcut to the ``yield_seeds`` method ----------------------------------------- +A shortcut to the ``start`` method +---------------------------------- -Instead of implementing a :meth:`~scrapy.Spider.yield_seeds` method that yields +Instead of implementing a :meth:`~scrapy.Spider.start` method that yields :class:`~scrapy.Request` objects from URLs, you can define a :attr:`~scrapy.Spider.start_urls` class attribute with a list of URLs. This list will then be used by the default implementation of -:meth:`~scrapy.Spider.yield_seeds` to create the initial requests for your +:meth:`~scrapy.Spider.start` to create the initial requests for your spider. .. code-block:: python @@ -795,7 +795,7 @@ with a specific tag, building the URL based on the argument: class QuotesSpider(scrapy.Spider): name = "quotes" - async def yield_seeds(self): + async def start(self): url = "https://quotes.toscrape.com/" tag = getattr(self, "tag", None) if tag is not None: diff --git a/docs/news.rst b/docs/news.rst index e35e8c2f7..190e73a9b 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -10,8 +10,8 @@ Scrapy VERSION (unreleased) Highlights: -- Replaced ``start_requests`` (sync) with :meth:`~scrapy.Spider.yield_seeds` - (async) and changed how it is iterated by default. +- Replaced ``start_requests()`` (sync) with :meth:`~scrapy.Spider.start` + (async) and changed how it is iterated. Modified requirements ~~~~~~~~~~~~~~~~~~~~~ @@ -23,17 +23,25 @@ Modified requirements Backward-incompatible changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- By default, the iteration of start requests and items no longer stops once - there are requests in the scheduler. +- The iteration of start requests and items no longer stops once there are + requests in the scheduler, and instead runs continuously until all start + requests have been scheduled. - You can restore the previous behavior by setting :setting:`SEEDING_POLICY` - to :py:enum:mem:`~scrapy.SeedingPolicy.lazy`. + As a result, the order in which start requests are sent may change. See + :ref:`start-requests` for details and information on how to force start + request order or :ref:`pause start request iteration while there are + scheduled requests `. + +- An unhandled exception from the + :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.open_spider` method of a + :ref:`spider middleware ` no longer stops the + crawl. - In ``scrapy.core.engine.ExecutionEngine``: - The second parameter of ``open_spider()``, ``start_requests``, has been - removed. The starting requests are determined by the ``spider`` - parameter instead (see :meth:`~scrapy.Spider.yield_seeds`). + removed. The start requests are determined by the ``spider`` parameter + instead (see :meth:`~scrapy.Spider.start`). - The ``slot`` attribute has been renamed to ``_slot`` and should not be used. @@ -44,21 +52,25 @@ Backward-incompatible changes - The ``slot`` :ref:`telnet variable ` has been removed. - In ``scrapy.core.spidermw.SpiderMiddlewareManager``, - ``process_start_requests()`` has been replaced by ``process_seeds()``. + ``process_start_requests()`` has been replaced by ``process_start()``. + +- The now-deprecated ``start_requests()`` method, when it returns an iterable + instead of being defined as a generator, is now executed *after* the + :ref:`scheduler ` instance has been created. Deprecations ~~~~~~~~~~~~ - The ``start_requests()`` method of :class:`~scrapy.Spider` is deprecated, - use :meth:`~scrapy.Spider.yield_seeds` instead, or both to maintain support - for lower Scrapy versions. + use :meth:`~scrapy.Spider.start` instead, or both to maintain support for + lower Scrapy versions. (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) - The ``process_start_requests()`` method of :ref:`spider middlewares ` is deprecated, use - :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_seeds` instead, or - both to maintain support for lower Scrapy versions. + :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_start` instead, + or both to maintain support for lower Scrapy versions. (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) @@ -66,9 +78,10 @@ New features ~~~~~~~~~~~~ - You can now yield the start requests and items of a spider from the - :meth:`~scrapy.Spider.yield_seeds` spider method and from the - :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_seeds` spider - middleware method, both asynchronous generators. + :meth:`~scrapy.Spider.start` spider method and from the + :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_start` spider + middleware method, both :term:`asynchronous generators `. This makes it possible to use asynchronous code to generate those start requests and items, e.g. reading them from a queue service or database @@ -76,20 +89,22 @@ New features (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) -- The new :setting:`SEEDING_POLICY` setting allows customizing how start - requests and items are iterated. +- Start requests are now :ref:`scheduled ` as soon as + possible. - You can also override the active seeding policy from - :meth:`Spider.yield_seeds ` and from - :meth:`SpiderMiddleware.process_seeds - `. + As a result, their :attr:`~scrapy.Request.priority` is now taken into + account as soon as :setting:`CONCURRENT_REQUESTS` is reached. - .. note:: Some third-party spider middlewares may need to be updated for - Scrapy VERSION support before you can use them in combination with the - ability to override the active seeding policy. + (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) - (:issue:`740`, :issue:`1051`, :issue:`1443`, :issue:`3237`, :issue:`4467`, - :issue:`5282`, :issue:`6730`) +- :class:`Crawler.signals ` has a new + :meth:`~scrapy.signalmanager.SignalManager.wait_for` method. + +- Added a new :signal:`scheduler_empty` signal. + +- Exposed a new method of :class:`Crawler.engine + `: + :meth:`~scrapy.core.engine.ExecutionEngine.needs_backout`. - You can now raise :exc:`~scrapy.exceptions.CloseSpider` from :meth:`~scrapy.Spider.yield_seeds` and from @@ -112,9 +127,10 @@ New features Bug fixes ~~~~~~~~~ -- Yielding a start item (i.e. from :meth:`~scrapy.Spider.yield_seeds` or an - equivalent) no longer delays the next iteration of starting requests and - items by up to 5 seconds. +- Yielding an item from :meth:`Spider.start ` or from + :meth:`SpiderMiddleware.process_start + ` no longer delays + the next iteration of starting requests and items by up to 5 seconds. (:issue:`6729`) @@ -128,7 +144,7 @@ Highlights: - Dropped support for Python 3.8, added support for Python 3.13 -- ``scrapy.Spider.start_requests`` can now yield items +- ``scrapy.Spider.start_requests()`` can now yield items - Added :class:`~scrapy.http.JsonResponse` @@ -419,9 +435,13 @@ Deprecations New features ~~~~~~~~~~~~ -- ``scrapy.Spider.start_requests`` can now yield items. +- ``scrapy.Spider.start_requests()`` can now yield items. (:issue:`5289`, :issue:`6417`) + .. note:: Some spider middlewares may need to be updated for Scrapy 2.12 + support before you can use them in combination with the ability to + yield items from ``start_requests()``. + - Added a new :class:`~scrapy.http.Response` subclass, :class:`~scrapy.http.JsonResponse`, for responses with a `JSON MIME type `_. @@ -911,7 +931,7 @@ Backward-incompatible changes in :meth:`scrapy.Spider.from_crawler`. If you want to access the final setting values and the initialized :class:`~scrapy.crawler.Crawler` attributes in the spider code as early as possible you can do this in - ``scrapy.Spider.start_requests`` or in a handler of the + ``scrapy.Spider.start_requests()`` or in a handler of the :signal:`engine_started` signal. (:issue:`6038`) - The :meth:`TextResponse.json ` method now @@ -3488,7 +3508,7 @@ New features * :class:`~scrapy.spiders.Spider` objects now raise an :exc:`AttributeError` exception if they do not have a :class:`~scrapy.spiders.Spider.start_urls` - attribute nor reimplement ``scrapy.spiders.Spider.start_requests``, + attribute nor reimplement ``scrapy.spiders.Spider.start_requests()``, but have a ``start_url`` attribute (:issue:`4133`, :issue:`4170`) * :class:`~scrapy.exporters.BaseItemExporter` subclasses may now use @@ -6409,7 +6429,7 @@ Scrapy 0.18.4 (released 2013-10-10) - IPython refuses to update the namespace. fix #396 (:commit:`3d32c4f`) - Fix AlreadyCalledError replacing a request in shell command. closes #407 (:commit:`b1d8919`) -- Fix ``start_requests`` laziness and early hangs (:commit:`89faf52`) +- Fix ``start_requests()`` laziness and early hangs (:commit:`89faf52`) Scrapy 0.18.3 (released 2013-10-03) ----------------------------------- @@ -6602,7 +6622,7 @@ Scrapy changes: - added options ``-o`` and ``-t`` to the :command:`runspider` command - documented :doc:`topics/autothrottle` and added to extensions installed by default. You still need to enable it with :setting:`AUTOTHROTTLE_ENABLED` - major Stats Collection refactoring: removed separation of global/per-spider stats, removed stats-related signals (``stats_spider_opened``, etc). Stats are much simpler now, backward compatibility is kept on the Stats Collector API and signals. -- added a ``process_start_requests`` method to spider middlewares +- added a ``process_start_requests()`` method to spider middlewares - dropped Signals singleton. Signals should now be accessed through the Crawler.signals attribute. See the signals documentation for more info. - dropped Stats Collector singleton. Stats can now be accessed through the Crawler.stats attribute. See the stats collection documentation for more info. - documented :ref:`topics-api` @@ -6665,7 +6685,7 @@ Scrapy 0.14.2 - fixed bug in MemoryUsage extension: get_engine_status() takes exactly 1 argument (0 given) (:commit:`11133e9`) - fixed struct.error on http compression middleware. closes #87 (:commit:`1423140`) - ajax crawling wasn't expanding for unicode urls (:commit:`0de3fb4`) -- Catch ``start_requests`` iterator errors. refs #83 (:commit:`454a21d`) +- Catch ``start_requests()`` iterator errors. refs #83 (:commit:`454a21d`) - Speed-up libxml2 XPathSelector (:commit:`2fbd662`) - updated versioning doc according to recent changes (:commit:`0a070f5`) - scrapyd: fixed documentation link (:commit:`2b4e4c3`) diff --git a/docs/requirements.txt b/docs/requirements.txt index 63243fcf3..103fb08d6 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,4 +1,3 @@ -enum-tools[sphinx]==0.12.0 sphinx==8.1.3 sphinx-hoverxref==1.4.2 sphinx-notfound-page==1.0.4 diff --git a/docs/topics/api.rst b/docs/topics/api.rst index 5a00fd570..8e8f3a0c9 100644 --- a/docs/topics/api.rst +++ b/docs/topics/api.rst @@ -280,3 +280,9 @@ class (which they all inherit from). Close the given spider. After this is called, no more specific stats can be accessed or collected. + +Engine API +========== + +.. autoclass:: scrapy.core.engine.ExecutionEngine() + :members: needs_backout diff --git a/docs/topics/architecture.rst b/docs/topics/architecture.rst index 4eff3cbe7..e8c510ea5 100644 --- a/docs/topics/architecture.rst +++ b/docs/topics/architecture.rst @@ -150,7 +150,7 @@ requests). Use a Spider middleware if you need to * post-process output of spider callbacks - change/add/remove requests or items; -* post-process seed requests or items; +* post-process start requests or items; * handle spider exceptions; * call errback instead of callback for some of the requests based on response content. diff --git a/docs/topics/coroutines.rst b/docs/topics/coroutines.rst index bed13f483..18aa8c68a 100644 --- a/docs/topics/coroutines.rst +++ b/docs/topics/coroutines.rst @@ -6,8 +6,8 @@ Coroutines .. versionadded:: 2.0 -Scrapy has :ref:`partial support ` for the :ref:`coroutine -syntax ` (i.e. ``async def``). +Scrapy :ref:`supports ` the :ref:`coroutine syntax ` +(i.e. ``async def``). .. _coroutine-support: @@ -18,7 +18,8 @@ Supported callables The following callables may be defined as coroutines using ``async def``, and hence use coroutine syntax (e.g. ``await``, ``async for``, ``async with``): -- The :meth:`~scrapy.spiders.Spider.yield_seeds` spider method. +- The :meth:`~scrapy.spiders.Spider.start` spider method, which *must* be + defined as an :term:`asynchronous generator`. .. versionadded: VERSION @@ -46,16 +47,17 @@ hence use coroutine syntax (e.g. ``await``, ``async for``, ``async with``): :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_spider_output` method of :ref:`spider middlewares `. - It must be defined as an :term:`asynchronous generator`. The input - ``result`` parameter is an :term:`asynchronous iterable`. + If defined as a coroutine, it must be an :term:`asynchronous generator`. + The input ``result`` parameter is an :term:`asynchronous iterable`. See also :ref:`sync-async-spider-middleware` and :ref:`universal-spider-middleware`. .. versionadded:: 2.7 -- The :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_seeds` method - of :ref:`spider middlewares `. +- The :meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_start` method + of :ref:`spider middlewares `, which *must* be + defined as an :term:`asynchronous generator`. .. versionadded:: VERSION @@ -157,7 +159,7 @@ This means you can use many useful Python libraries providing such code: Common use cases for asynchronous code include: * requesting data from websites, databases and other services (in - :meth:`~scrapy.spiders.Spider.yield_seeds`, callbacks, pipelines and + :meth:`~scrapy.spiders.Spider.start`, callbacks, pipelines and middlewares); * storing data in databases (in pipelines and middlewares); * delaying the spider initialization until some external event (in the diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index de7840878..a98f77df5 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -352,7 +352,7 @@ errors if needed: "https://example.invalid/", # DNS error expected ] - async def yield_seeds(self): + async def start(self): for u in self.start_urls: yield scrapy.Request( u, diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 4d573a9f6..0407e4083 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1751,29 +1751,6 @@ Soft limit (in bytes) for response data being processed. While the sum of the sizes of all responses being processed is above this value, Scrapy does not process new requests. -.. setting:: SEEDING_POLICY - -SEEDING_POLICY --------------- - -.. versionadded:: VERSION - -Default: :py:enum:mem:`SeedingPolicy.greedy ` - -Determines the way :meth:`Spider.yield_seeds ` is -iterated. - -Its value may be defined as a member of the :class:`~scrapy.SeedingPolicy` enum -(e.g. :py:enum:mem:`SeedingPolicy.lazy `) or as a -matching string (e.g. ``"lazy"``). - -You can also override the active seeding policy from :meth:`Spider.yield_seeds -` and from :meth:`SpiderMiddleware.process_seeds -`. - -.. autoenum:: scrapy.SeedingPolicy - :members: - .. setting:: SPIDER_CONTRACTS SPIDER_CONTRACTS @@ -1991,7 +1968,7 @@ In order to use the reactor installed by Scrapy: self.timeout = int(kwargs.pop("timeout", "60")) super(QuotesSpider, self).__init__(*args, **kwargs) - async def yield_seeds(self): + async def start(self): reactor.callLater(self.timeout, self.stop) urls = ["https://quotes.toscrape.com/page/1"] @@ -2020,7 +1997,7 @@ which raises :exc:`Exception`, becomes: self.timeout = int(kwargs.pop("timeout", "60")) super(QuotesSpider, self).__init__(*args, **kwargs) - async def yield_seeds(self): + async def start(self): from twisted.internet import reactor reactor.callLater(self.timeout, self.stop) @@ -2088,6 +2065,21 @@ also used by :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware if :setting:`ROBOTSTXT_USER_AGENT` setting is ``None`` and there is no overriding User-Agent header specified for the request. +.. setting:: WARN_ON_GENERATOR_RETURN_VALUE + +WARN_ON_GENERATOR_RETURN_VALUE +------------------------------ + +Default: ``True`` + +When enabled, Scrapy will warn if generator-based callback methods (like +``parse``) contain return statements with non-``None`` values. This helps detect +potential mistakes in spider development. + +Disable this setting to prevent syntax errors that may occur when dynamically +modifying generator function source code during runtime, skip AST parsing of +callback functions, or improve performance in auto-reloading development +environments. Settings documented elsewhere: ------------------------------ diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 782ac1e26..66cb87fc5 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -131,6 +131,19 @@ engine_stopped This signal supports returning deferreds from its handlers. +scheduler_empty +~~~~~~~~~~~~~~~ + +.. signal:: scheduler_empty +.. function:: scheduler_empty() + + Sent whenever the engine asks for a pending request from the + :ref:`scheduler ` (i.e. calls its + :meth:`~scrapy.core.scheduler.BaseScheduler.next_request` method) and the + scheduler returns none. + + See :ref:`start-requests-lazy` for an example. + Item signals ------------ @@ -160,7 +173,7 @@ item_scraped :type spider: :class:`~scrapy.Spider` object :param response: the response from where the item was scraped, or ``None`` - if it was yielded from :meth:`~scrapy.Spider.yield_seeds`. + if it was yielded from :meth:`~scrapy.Spider.start`. :type response: :class:`~scrapy.http.Response` | ``None`` item_dropped @@ -181,7 +194,7 @@ item_dropped :type spider: :class:`~scrapy.Spider` object :param response: the response from where the item was dropped, or ``None`` - if it was yielded from :meth:`~scrapy.Spider.yield_seeds`. + if it was yielded from :meth:`~scrapy.Spider.start`. :type response: :class:`~scrapy.http.Response` | ``None`` :param exception: the exception (which must be a @@ -205,7 +218,7 @@ item_error :param response: the response being processed when the exception was raised, or ``None`` if it was yielded from - :meth:`~scrapy.Spider.yield_seeds`. + :meth:`~scrapy.Spider.start`. :type response: :class:`~scrapy.http.Response` | ``None`` :param spider: the spider which raised the exception diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index daaa28fa0..760abd3f4 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -70,39 +70,21 @@ one or more of these methods: .. class:: SpiderMiddleware - .. method:: process_seeds(seeds: AsyncIterator[Any], /) -> AsyncIterator[Any] + .. method:: process_start(start: AsyncIterator[Any], /) -> AsyncIterator[Any] :async: - Iterate over the output of :meth:`~scrapy.Spider.yield_seeds` or that - of the :meth:`process_seeds` method of an earlier spider middleware, + Iterate over the output of :meth:`~scrapy.Spider.start` or that + of the :meth:`process_start` method of an earlier spider middleware, overriding it. For example: .. code-block:: python - async def process_seeds(self, seeds): - async for seed in seeds: - yield seed + async def process_start(self, start): + async for item_or_request in start: + yield item_or_request - You may yield the same type of objects as - :meth:`~scrapy.Spider.yield_seeds`. It may also raise - :exc:`~scrapy.exceptions.CloseSpider`. - - As with :meth:`~scrapy.Spider.yield_seeds`, how this method is iterated - by default is controlled by :setting:`SEEDING_POLICY`. It is also - possible to yield a :class:`~scrapy.SeedingPolicy` enum or a matching - string to change the active seeding policy, for example: - - .. code-block:: python - - async def process_seeds(self, seeds): - yield "front_load" - async for seed in seeds: - yield seed - yield "idle" - - .. tip:: You can also restore the configured seeding policy by - :ref:`reading its value ` from the - :setting:`SEEDING_POLICY` setting and yielding it. + You may yield the same type of objects as :meth:`~scrapy.Spider.start`. + You may also raise :exc:`~scrapy.exceptions.CloseSpider`. To write spider middlewares that work on Scrapy versions lower than VERSION, define also a synchronous ``process_start_requests()`` method @@ -110,8 +92,8 @@ one or more of these methods: .. code-block:: python - def process_start_requests(self, seeds, spider): - yield from seeds + def process_start_requests(self, start, spider): + yield from start .. method:: process_spider_input(response, spider) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 88eed477a..ae80f347d 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -17,7 +17,7 @@ For spiders, the scraping cycle goes through something like this: those requests. The first requests to perform are obtained by iterating the - :meth:`~scrapy.Spider.yield_seeds` method, which by default yields a + :meth:`~scrapy.Spider.start` method, which by default yields a :class:`~scrapy.Request` object for each URL in the :attr:`~scrapy.Spider.start_urls` spider attribute, with the :attr:`~scrapy.Spider.parse` method set as :attr:`~scrapy.Request.callback` @@ -137,7 +137,7 @@ scrapy.Spider The final settings and the initialized :class:`~scrapy.crawler.Crawler` attributes are available in the - :meth:`yield_seeds` method, handlers of the + :meth:`start` method, handlers of the :signal:`engine_started` signal and later. :param crawler: crawler to which the spider will be bound @@ -189,7 +189,7 @@ scrapy.Spider super().update_settings(settings) settings.setdefault("FEEDS", {}).update(cls.custom_feed) - .. automethod:: yield_seeds + .. automethod:: start .. method:: parse(response) @@ -261,8 +261,9 @@ Return multiple Requests and items from a single callback: for href in response.xpath("//a/@href").getall(): yield scrapy.Request(response.urljoin(href), self.parse) -Instead of :attr:`~.start_urls` you can use :meth:`~.yield_seeds` directly; -to give data more structure you can use :class:`~scrapy.Item` objects: +Instead of :attr:`~.start_urls` you can use :meth:`~scrapy.Spider.start` +directly; to give data more structure you can use :class:`~scrapy.Item` +objects: .. skip: next .. code-block:: python @@ -275,7 +276,7 @@ to give data more structure you can use :class:`~scrapy.Item` objects: name = "example.com" allowed_domains = ["example.com"] - async def yield_seeds(self): + async def start(self): yield scrapy.Request("http://www.example.com/1.html", self.parse) yield scrapy.Request("http://www.example.com/2.html", self.parse) yield scrapy.Request("http://www.example.com/3.html", self.parse) @@ -329,7 +330,7 @@ The above example can also be written as follows: class MySpider(scrapy.Spider): name = "myspider" - async def yield_seeds(self): + async def start(self): yield scrapy.Request(f"http://www.example.com/categories/{self.category}") If you are :ref:`running Scrapy from a script `, you can @@ -363,6 +364,96 @@ used by :class:`~scrapy.downloadermiddlewares.useragent.UserAgentMiddleware`:: Spider arguments can also be passed through the Scrapyd ``schedule.json`` API. See `Scrapyd documentation`_. +.. _start-requests: + +Start requests +============== + +Scrapy does not try to send :meth:`~scrapy.Spider.start` requests in order. +Instead, it prioritizes reaching :setting:`CONCURRENT_REQUESTS` and +:ref:`scheduling ` start requests. + +.. + The request send order when all start requests and callback requests have + the same priority is rather unintuitive: + + 1. First, the first CONCURRENT_REQUESTS start requests are sent in order. + + Awaiting slow operations in Spider.start() can lower that. + + 2. Then, assuming an even domain distribution in start requests (i.e. + ABCABC, not AABBCC), the last N start requests are sent in reverse + order, where N is: + + min(CONCURRENT_REQUESTS, CONCURRENT_REQUESTS_PER_DOMAIN * domain_count) + + 3. Finally, the remaining start requests are also sent in reverse order, + but only when there are not enough pending requests yielded from + callbacks to reach the configured concurrency. + + The reverse order is because the scheduler uses a LIFO queue by default + (SCHEDULER_MEMORY_QUEUE, SCHEDULER_DISK_QUEUE). The order of the first few + requests is unnaffected because they are sent as soon as they are + scheduled. The last start requests sent before callback requests are those + that can be sent before the first callback requests are scheduled. + + We do not document this behavior, so that we may change it in the future + without breaking the contract. + +.. _start-requests-order: + +Forcing a start request order +----------------------------- + +To force a specific **request order**, override the +:meth:`~scrapy.Spider.start` method to set :attr:`Request.priority +`. For example: + +- To send start requests before other requests: + + .. code-block:: python + + async def start(self): + async for item_or_request in super().start(): + if isinstance(item_or_request, Request): + item_or_request = item_or_request.replace(priority=1) + yield item_or_request + +- To send start requests in order: + + .. code-block:: python + + async def start(self): + priority = len(self.start_urls) + async for item_or_request in super().start(): + if isinstance(item_or_request, Request): + item_or_request = item_or_request.replace(priority=priority) + yield item_or_request + priority -= 1 + +You can also :ref:`customize the scheduler ` if you need +more control over request prioritization. + +.. _start-requests-lazy: + +Delaying start request iteration +-------------------------------- + +You can override the :meth:`~scrapy.Spider.start` method as follows to pause +its iteration whenever there are scheduled requests: + +.. code-block:: python + + async def start(self): + async for item_or_request in super().start(): + if self.crawler.engine.needs_backoff(): + await self.crawler.signals.wait_for(signals.scheduler_empty) + yield item_or_request + +This can help minimize the number of requests in the scheduler at any given +time, to minimize resource usage (memory or disk, depending on +:setting:`JOBDIR`). + .. _builtin-spiders: Generic Spiders @@ -893,9 +984,9 @@ Combine SitemapSpider with other sources of urls: other_urls = ["http://www.example.com/about"] - async def yield_seeds(self): - async for seed in super().yield_seeds(): - yield seed + async def start(self): + async for item_or_request in super().start(): + yield item_or_request for url in self.other_urls: yield Request(url, self.parse_other) diff --git a/extras/qpsclient.py b/extras/qpsclient.py index df1bf8767..269b27336 100644 --- a/extras/qpsclient.py +++ b/extras/qpsclient.py @@ -34,9 +34,9 @@ class QPSSpider(Spider): elif self.download_delay is not None: self.download_delay = float(self.download_delay) - async def yield_seeds(self): - for seed in self.start_requests(): - yield seed + async def start(self): + for item_or_request in self.start_requests(): + yield item_or_request def start_requests(self): url = self.benchurl diff --git a/scrapy/__init__.py b/scrapy/__init__.py index 7bb958a2d..256504c9c 100644 --- a/scrapy/__init__.py +++ b/scrapy/__init__.py @@ -7,7 +7,6 @@ import sys import warnings # Declare top-level shortcuts -from scrapy.core._seeding import SeedingPolicy from scrapy.http import FormRequest, Request from scrapy.item import Field, Item from scrapy.selector import Selector @@ -18,7 +17,6 @@ __all__ = [ "FormRequest", "Item", "Request", - "SeedingPolicy", "Selector", "Spider", "__version__", diff --git a/scrapy/commands/bench.py b/scrapy/commands/bench.py index fccf7c2d3..96bb1ae84 100644 --- a/scrapy/commands/bench.py +++ b/scrapy/commands/bench.py @@ -59,7 +59,7 @@ class _BenchSpider(scrapy.Spider): baseurl = "http://localhost:8998" link_extractor = LinkExtractor() - async def yield_seeds(self) -> AsyncIterator[Any]: + async def start(self) -> AsyncIterator[Any]: qargs = {"total": self.total, "show": self.show} url = f"{self.baseurl}?{urlencode(qargs, doseq=True)}" yield scrapy.Request(url, dont_filter=True) diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py index 24dfc0106..56dc1ea55 100644 --- a/scrapy/commands/check.py +++ b/scrapy/commands/check.py @@ -80,16 +80,14 @@ class Command(ScrapyCommand): assert self.crawler_process spider_loader = self.crawler_process.spider_loader - async def yield_seeds(self): - for request in conman.from_spider(self, self._result): + async def start(self): + for request in conman.from_spider(self, result): yield request with set_environ(SCRAPY_CHECK="true"): for spidername in args or spider_loader.list(): spidercls = spider_loader.load(spidername) - - spidercls._result = result # type: ignore[assignment,attr-defined,method-assign,return-value] - spidercls.yield_seeds = yield_seeds # type: ignore[assignment,method-assign,return-value] + spidercls.start = start # type: ignore[assignment,method-assign,return-value] tested_methods = conman.tested_methods_from_spidercls(spidercls) if opts.list: @@ -107,10 +105,10 @@ class Command(ScrapyCommand): for method in sorted(methods): print(f" * {method}") else: - start = time.time() + start_time = time.time() self.crawler_process.start() stop = time.time() result.printErrors() - result.printSummary(start, stop) + result.printSummary(start_time, stop) self.exitcode = int(not result.wasSuccessful()) diff --git a/scrapy/commands/fetch.py b/scrapy/commands/fetch.py index d1b0974ec..ef6e13de2 100644 --- a/scrapy/commands/fetch.py +++ b/scrapy/commands/fetch.py @@ -90,11 +90,10 @@ class Command(ScrapyCommand): else: spidercls = spidercls_for_request(spider_loader, request, spidercls) - async def yield_seeds(self): - yield self._request + async def start(self): + yield request - spidercls._request = request # type: ignore[assignment,attr-defined] - spidercls.yield_seeds = yield_seeds # type: ignore[method-assign,attr-defined] + spidercls.start = start # type: ignore[method-assign,attr-defined] self.crawler_process.crawl(spidercls) self.crawler_process.start() diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index f0d152570..0dd9954cb 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -258,11 +258,11 @@ class Command(BaseRunSpiderCommand): if not self.spidercls: logger.error("Unable to find spider for: %(url)s", {"url": url}) - async def yield_seeds(spider: Spider) -> AsyncIterator[Any]: + async def start(spider: Spider) -> AsyncIterator[Any]: yield self.prepare_request(spider, Request(url), opts) if self.spidercls: - self.spidercls.yield_seeds = yield_seeds # type: ignore[assignment,method-assign] + self.spidercls.start = start # type: ignore[assignment,method-assign] def start_parsing(self, url: str, opts: argparse.Namespace) -> None: assert self.crawler_process diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index c50c963ef..9dabfcd9c 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -9,7 +9,6 @@ from __future__ import annotations from threading import Thread from typing import TYPE_CHECKING, Any -from scrapy import SeedingPolicy from scrapy.commands import ScrapyCommand from scrapy.http import Request from scrapy.shell import Shell @@ -28,7 +27,6 @@ class Command(ScrapyCommand): "DUPEFILTER_CLASS": "scrapy.dupefilters.BaseDupeFilter", "KEEP_ALIVE": True, "LOGSTATS_INTERVAL": 0, - "SEEDING_POLICY": SeedingPolicy.lazy, } def syntax(self) -> str: @@ -87,7 +85,7 @@ class Command(ScrapyCommand): crawler._apply_settings() # The Shell class needs a persistent engine in the crawler crawler.engine = crawler._create_engine() - crawler.engine.start() + crawler.engine.start(_start_request_processing=False) self._start_crawler_thread() diff --git a/scrapy/core/_seeding.py b/scrapy/core/_seeding.py deleted file mode 100644 index 83f3f4159..000000000 --- a/scrapy/core/_seeding.py +++ /dev/null @@ -1,65 +0,0 @@ -from enum import Enum - -try: - from enum_tools.documentation import document_enum -except ImportError: - - def document_enum(func): # type: ignore[misc] - return func -else: - # https://github.com/domdfcoding/enum_tools/issues/29 - import enum_tools.documentation - - enum_tools.documentation.INTERACTIVE = True - - -@document_enum -class SeedingPolicy(Enum): - front_load = "front_load" - """The crawl does not start until all seed requests have been scheduled. - - Aims to give the :ref:`scheduler ` full control over - request order from the start. Some custom schedulers may require this - seeding policy to work as designed. - """ - - greedy = "greedy" - """Iterating seeds takes priority over processing scheduled requests. - - Every time a seed request is iterated, it is scheduled, and then the next - request from the scheduler is sent. - - .. note:: That request sent may not be the scheduled seed request - depending on the priority of scheduled requests, on the configured - :setting:`SCHEDULER` and on certain scheduler settings (e.g. - :setting:`SCHEDULER_MEMORY_QUEUE`). - - Best used when prioritizing seed requests is important. - """ - - idle = "idle" - """A single seed is read only when there are neither scheduled nor on-going - requests. - - That is, a new seed is not read until all requests triggered by the - previous seed, directly or indirectly, have been processed. - - Unlike :py:enum:mem:`lazy`, resource savings are prioritized over crawl - speed. - - It is functionally equivalent to running a spider multiple times in a row, - one per seed request. - """ - - lazy = "lazy" - """Processing scheduled requests takes priority over iterating seeds. - - Aims to minimize the number of requests in the scheduler at any given time, - to minimize resource usage (memory or disk, depending on - :setting:`JOBDIR`). - - It is best used when seed request priority is not important. - - Switching to :py:enum:mem:`idle` may lower resource usage further at the - cost of also lowering crawl speed. - """ diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index af4df161f..565f61919 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -12,23 +12,24 @@ from time import time from traceback import format_exc from typing import TYPE_CHECKING, Any, TypeVar, cast -from twisted.internet.defer import Deferred, inlineCallbacks, succeed +from twisted.internet.defer import Deferred, succeed from twisted.python.failure import Failure from scrapy import signals from scrapy.core.scraper import Scraper, _HandleOutputDeferred from scrapy.exceptions import CloseSpider, DontCloseSpider, IgnoreRequest from scrapy.http import Request, Response -from scrapy.utils.defer import deferred_from_coro +from scrapy.utils.defer import ( + deferred_f_from_coro_f, + maybe_deferred_to_future, +) from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.python import global_object_name from scrapy.utils.reactor import CallLaterOnce -from ._seeding import SeedingPolicy - if TYPE_CHECKING: - from collections.abc import AsyncIterator, Callable, Generator + from collections.abc import AsyncIterator, Callable from scrapy.core.downloader import Downloader from scrapy.core.scheduler import BaseScheduler @@ -44,21 +45,17 @@ logger = logging.getLogger(__name__) _T = TypeVar("_T") -class _SeedingPolicyChange(Exception): - pass - - class _Slot: def __init__( self, close_if_idle: bool, - nextcall: CallLaterOnce[Deferred[None]], + nextcall: CallLaterOnce[None], scheduler: BaseScheduler, ) -> None: self.closing: Deferred[None] | None = None self.inprogress: set[Request] = set() self.close_if_idle: bool = close_if_idle - self.nextcall: CallLaterOnce[Deferred[None]] = nextcall + self.nextcall: CallLaterOnce[None] = nextcall self.scheduler: BaseScheduler = scheduler def add_request(self, request: Request) -> None: @@ -98,31 +95,22 @@ class ExecutionEngine: self.spider: Spider | None = None self.running: bool = False self.paused: bool = False - self.scheduler_cls: type[BaseScheduler] = self._get_scheduler_class( - crawler.settings - ) - downloader_cls: type[Downloader] = load_object(self.settings["DOWNLOADER"]) - self.downloader: Downloader = downloader_cls(crawler) - self.scraper: Scraper = Scraper(crawler) self._spider_closed_callback: Callable[[Spider], Deferred[None] | None] = ( spider_closed_callback ) self.start_time: float | None = None - self._load_seeding_policy() - self._seeds: AsyncIterator[Any] | None = None - self._waiting_for_seed: bool = False + self._start: AsyncIterator[Any] | None = None + downloader_cls: type[Downloader] = load_object(self.settings["DOWNLOADER"]) self._back_in_seconds = self._MIN_BACK_IN_SECONDS - - def _load_seeding_policy(self) -> None: try: - self._seeding_policy = SeedingPolicy(self.settings["SEEDING_POLICY"]) - except ValueError: - supported_values = ", ".join(policy.value for policy in SeedingPolicy) - raise ValueError( - f"The value of the SEEDING_POLICY setting " - f"({self.settings['SEEDING_POLICY']!r}) is not supported. " - f"Supported values: {supported_values}." + self.scheduler_cls: type[BaseScheduler] = self._get_scheduler_class( + crawler.settings ) + self.downloader: Downloader = downloader_cls(crawler) + self.scraper: Scraper = Scraper(crawler) + except Exception: + self.close() + raise def _get_scheduler_class(self, settings: BaseSettings) -> type[BaseScheduler]: from scrapy.core.scheduler import BaseScheduler @@ -135,22 +123,28 @@ class ExecutionEngine: ) return scheduler_cls - @inlineCallbacks - def start(self) -> Generator[Deferred[Any], Any, None]: + @deferred_f_from_coro_f + async def start(self, _start_request_processing=True) -> None: if self.running: raise RuntimeError("Engine already running") self.start_time = time() - yield self.signals.send_catch_log_deferred(signal=signals.engine_started) + await maybe_deferred_to_future( + self.signals.send_catch_log_deferred(signal=signals.engine_started) + ) self.running = True self._closewait: Deferred[None] = Deferred() - yield self._closewait + if _start_request_processing: + self._start_request_processing() + await maybe_deferred_to_future(self._closewait) def stop(self) -> Deferred[None]: """Gracefully stop the execution engine""" - @inlineCallbacks - def _finish_stopping_engine(_: Any) -> Generator[Deferred[Any], Any, None]: - yield self.signals.send_catch_log_deferred(signal=signals.engine_stopped) + @deferred_f_from_coro_f + async def _finish_stopping_engine(_: Any) -> None: + await maybe_deferred_to_future( + self.signals.send_catch_log_deferred(signal=signals.engine_stopped) + ) self._closewait.callback(None) if not self.running: @@ -184,70 +178,39 @@ class ExecutionEngine: def unpause(self) -> None: self.paused = False - @inlineCallbacks - def _process_next_seed(self): - if self._waiting_for_seed: - return - self._waiting_for_seed = True - # Schedule a new call for next requests while waiting for - # self._seeds.__anext__(), so that if it takes long enough and there - # are pending scheduler requests we can process those. - self._slot.nextcall.schedule(self._MIN_BACK_IN_SECONDS) + async def _process_start_next(self) -> None: + """Processes the next item or request from Spider.start(). + + If a request, it is scheduled. If an item, it is sent to item + pipelines. + """ + assert self._start is not None try: - seed = yield deferred_from_coro(self._seeds.__anext__()) + item_or_request = await self._start.__anext__() except StopAsyncIteration: - self._seeds = None - except CloseSpider: - self._seeds = None - raise + self._start = None + except CloseSpider as exception: + if self.spider: + await maybe_deferred_to_future( + self.close_spider(self.spider, reason=exception.reason) + ) + return except Exception as exception: - self._seeds = None + self._start = None exception_traceback = format_exc() logger.error( - f"Error while reading seeds: {exception}.\n{exception_traceback}" + f"Error while reading start items and requests: {exception}.\n{exception_traceback}", + exc_info=True, ) else: - if isinstance(seed, Request): - self.crawl(seed) - if ( - self._seeding_policy is not SeedingPolicy.front_load - and not self._needs_backout() - ): - self._start_scheduled_request() - elif isinstance(seed, (str, SeedingPolicy)): - try: - self._seeding_policy = SeedingPolicy(seed) - except ValueError: - valid_policy_strings = ", ".join( - policy.value for policy in SeedingPolicy - ) - logger.error( - f"Seed {seed!r} has been ignored. Seeds of {str} type " - f"must be valid seeding policies " - f"({valid_policy_strings})." - ) - self._slot.nextcall.schedule() - else: - raise _SeedingPolicyChange + if not self.spider: + return # spider already closed + if isinstance(item_or_request, Request): + self.crawl(item_or_request) else: - self.scraper.start_itemproc(seed, response=None) + self.scraper.start_itemproc(item_or_request, response=None) + assert self._slot is not None # typing self._slot.nextcall.schedule() - finally: - self._waiting_for_seed = False - if self._seeding_policy is SeedingPolicy.front_load and self._seeds is None: - self._slot.nextcall.schedule() - - def _process_scheduler_requests(self): - while not self._needs_backout(): - if self._start_scheduled_request() is None: - break - - def _can_process_next_seed(self) -> bool: - return ( - not self._waiting_for_seed - and self._seeds is not None - and not self._needs_backout() - ) def _scheduler_has_pending_requests(self) -> bool: assert self._slot is not None # typing @@ -256,48 +219,47 @@ class ExecutionEngine: except Exception as exception: exception_traceback = format_exc() logger.error( - f"{global_object_name(self._slot.scheduler.has_pending_requests)} raised an exception: {exception}.\n{exception_traceback}" + f"{global_object_name(self._slot.scheduler.has_pending_requests)} raised an exception: {exception}.\n{exception_traceback}", + exc_info=True, ) return False - @inlineCallbacks - def _run_loop(self) -> Generator[Deferred[Any], Any, None]: - """Sends new requests from seeds or from the scheduler based on the - configured seeding policy and on whether or not there is room for them, - e.g. on whether there are fewer active requests than the configured - maximum concurrency or if the total size of responses being processed - is below the configured limit.""" + async def _wait_until_next_loop_iteration(self) -> None: + from twisted.internet import reactor + + deferred: Deferred[None] = Deferred() + reactor.callLater(0, deferred.callback, None) + await maybe_deferred_to_future(deferred) + + @deferred_f_from_coro_f + async def _start_request_processing(self) -> None: + """Starts consuming Spider.start() output and sending scheduled + requests.""" + # Starts the processing of scheduled requests, as well as a periodic + # call to that processing method for scenarios where the scheduler + # reports having pending requests but returns none. + assert self._slot is not None # typing + self._slot.nextcall.schedule() + + while self._start and self.spider: + await self._process_start_next() + if not self.needs_backout(): + # Give room for the outcome of self._start_scheduled_requests() + # to be processed before continuing with the next iteration. + self._slot.nextcall.schedule() + await self._wait_until_next_loop_iteration() + + def _start_scheduled_requests(self) -> None: if self._slot is None or self._slot.closing is not None or self.paused: return - try: - if self._seeding_policy in {SeedingPolicy.idle, SeedingPolicy.lazy}: - self._process_scheduler_requests() - if self._can_process_next_seed() and ( - self._seeding_policy is not SeedingPolicy.idle - or not self.downloader.active - ): - yield self._process_next_seed() - else: - assert self._seeding_policy in { - SeedingPolicy.front_load, - SeedingPolicy.greedy, - } - if self._can_process_next_seed(): - yield self._process_next_seed() - else: - self._process_scheduler_requests() - except _SeedingPolicyChange: - self._slot.nextcall.schedule() - return - except CloseSpider as exception: - assert self.spider is not None # typing - self.close_spider(self.spider, reason=exception.reason) - return + while not self.needs_backout(): + if self._start_scheduled_request() is None: + break if self.spider_is_idle() and self._slot.close_if_idle: self._spider_idle() - elif self._needs_backout(): + elif self.needs_backout(): self._back_in_seconds = self._MIN_BACK_IN_SECONDS elif self._scheduler_has_pending_requests(): # If the scheduler reports having pending requests but did not @@ -310,7 +272,12 @@ class ExecutionEngine: self._back_in_seconds**2, self._MAX_BACK_IN_SECONDS ) - def _needs_backout(self) -> bool: + def needs_backout(self) -> bool: + """Returns ``True`` if no more requests can be sent at the moment, or + ``False`` otherwise. + + See :ref:`start-requests-lazy` for an example. + """ assert self._slot is not None # typing assert self.scraper.slot is not None # typing return ( @@ -333,6 +300,7 @@ class ExecutionEngine: ) return None if request is None: + self.signals.send_catch_log(signals.scheduler_empty) return None d: Deferred[Response | Request] = self._download(request) @@ -400,7 +368,7 @@ class ExecutionEngine: return False if self.downloader.active: # downloader has pending requests return False - if self._seeds is not None: # not all start requests are handled + if self._start is not None: # not all start requests are handled return False return not self._scheduler_has_pending_requests() @@ -489,27 +457,30 @@ class ExecutionEngine: dwld.addBoth(_on_complete) return dwld - @inlineCallbacks - def open_spider( + @deferred_f_from_coro_f + async def open_spider( self, spider: Spider, close_if_idle: bool = True, - ) -> Generator[Deferred[Any], Any, None]: + ) -> None: if self._slot is not None: raise RuntimeError(f"No free spider slot when opening {spider.name!r}") logger.info("Spider opened", extra={"spider": spider}) - nextcall = CallLaterOnce(self._run_loop) - scheduler = build_from_crawler(self.scheduler_cls, self.crawler) - self._seeds = yield self.scraper.spidermw.process_seeds(spider) - self._slot = _Slot(close_if_idle, nextcall, scheduler) self.spider = spider + nextcall = CallLaterOnce(self._start_scheduled_requests) + scheduler = build_from_crawler(self.scheduler_cls, self.crawler) + self._slot = _Slot(close_if_idle, nextcall, scheduler) + self._start = await maybe_deferred_to_future( + self.scraper.spidermw.process_start(spider) + ) if hasattr(scheduler, "open") and (d := scheduler.open(spider)): - yield d - yield self.scraper.open_spider(spider) + await maybe_deferred_to_future(d) + await maybe_deferred_to_future(self.scraper.open_spider(spider)) assert self.crawler.stats self.crawler.stats.open_spider(spider) - yield self.signals.send_catch_log_deferred(signals.spider_opened, spider=spider) - self._slot.nextcall.schedule() + await maybe_deferred_to_future( + self.signals.send_catch_log_deferred(signals.spider_opened, spider=spider) + ) def _spider_idle(self) -> None: """ diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index b664b61f6..85dd9b559 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -5,7 +5,7 @@ from __future__ import annotations import logging from collections import deque -from collections.abc import AsyncIterable, Iterator +from collections.abc import AsyncIterator, Iterator from typing import TYPE_CHECKING, Any, TypeVar, Union, cast from twisted.internet.defer import Deferred, inlineCallbacks @@ -170,7 +170,7 @@ class Scraper: raise TypeError( f"Incorrect type: expected Response or Failure, got {type(result)}: {result!r}" ) - dfd: Deferred[Iterable[Any] | AsyncIterable[Any]] = self._scrape2( + dfd: Deferred[Iterable[Any] | AsyncIterator[Any]] = self._scrape2( result, request, spider ) # returns spider's processed output dfd.addErrback(self.handle_spider_error, request, result, spider) @@ -181,7 +181,7 @@ class Scraper: def _scrape2( self, result: Response | Failure, request: Request, spider: Spider - ) -> Deferred[Iterable[Any] | AsyncIterable[Any]]: + ) -> Deferred[Iterable[Any] | AsyncIterator[Any]]: """ Handle the different cases of request's result been a Response or a Failure """ @@ -197,7 +197,7 @@ class Scraper: def call_spider( self, result: Response | Failure, request: Request, spider: Spider - ) -> Deferred[Iterable[Any] | AsyncIterable[Any]]: + ) -> Deferred[Iterable[Any] | AsyncIterator[Any]]: dfd: Deferred[Any] if isinstance(result, Response): if getattr(result, "request", None) is None: @@ -216,7 +216,7 @@ class Scraper: if request.errback: warn_on_generator_with_return_value(spider, request.errback) dfd.addErrback(request.errback) - dfd2: Deferred[Iterable[Any] | AsyncIterable[Any]] = dfd.addCallback( + dfd2: Deferred[Iterable[Any] | AsyncIterator[Any]] = dfd.addCallback( iterate_spider_output ) return dfd2 @@ -246,22 +246,23 @@ class Scraper: spider=spider, ) assert self.crawler.stats + self.crawler.stats.inc_value("spider_exceptions/count", spider=spider) self.crawler.stats.inc_value( f"spider_exceptions/{_failure.value.__class__.__name__}", spider=spider ) def handle_spider_output( self, - result: Iterable[_T] | AsyncIterable[_T], + result: Iterable[_T] | AsyncIterator[_T], request: Request, response: Response, spider: Spider, ) -> _HandleOutputDeferred: if not result: return defer_succeed(None) - it: Iterable[_T] | AsyncIterable[_T] + it: Iterable[_T] | AsyncIterator[_T] dfd: Deferred[_ParallelResult] - if isinstance(result, AsyncIterable): + if isinstance(result, AsyncIterator): it = aiter_errback( result, self.handle_spider_error, request, response, spider ) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index f94b8025f..14fc29fc7 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -7,7 +7,7 @@ See documentation in docs/topics/spider-middleware.rst from __future__ import annotations import logging -from collections.abc import AsyncIterable, AsyncIterator, Callable, Iterable +from collections.abc import AsyncIterator, Callable, Iterable from inspect import isasyncgenfunction, iscoroutine from itertools import islice from typing import TYPE_CHECKING, Any, TypeVar, Union, cast @@ -41,12 +41,12 @@ logger = logging.getLogger(__name__) _T = TypeVar("_T") ScrapeFunc = Callable[ - [Union[Response, Failure], Request, Spider], Union[Iterable[_T], AsyncIterable[_T]] + [Union[Response, Failure], Request, Spider], Union[Iterable[_T], AsyncIterator[_T]] ] def _isiterable(o: Any) -> bool: - return isinstance(o, (Iterable, AsyncIterable)) + return isinstance(o, (Iterable, AsyncIterator)) class SpiderMiddlewareManager(MiddlewareManager): @@ -67,8 +67,27 @@ class SpiderMiddlewareManager(MiddlewareManager): middleware for middleware in middlewares if hasattr(middleware, "process_start_requests") - and not hasattr(middleware, "process_seeds") + and not hasattr(middleware, "process_start") ] + modern_middlewares = [ + middleware + for middleware in middlewares + if not hasattr(middleware, "process_start_requests") + and hasattr(middleware, "process_start") + ] + if deprecated_middlewares and modern_middlewares: + raise ValueError( + "You are trying to combine spider middlewares that only " + "define the deprecated process_start_requests() method () " + "with spider middlewares that only define the " + "process_start() method (). This is not possible. You must " + "either disable or make universal 1 of those 2 sets of " + "spider middlewares. Making a spider middleware universal " + "means having it define both methods. See the release notes " + "of Scrapy VERSION for details: " + "https://docs.scrapy.org/en/VERSION/news.html" + ) + self._use_start_requests = bool(deprecated_middlewares) if self._use_start_requests: deprecated_middleware_list = ", ".join( @@ -80,16 +99,16 @@ class SpiderMiddlewareManager(MiddlewareManager): f"through their parent classes, define the deprecated " f"process_start_requests() method: " f"{deprecated_middleware_list}. process_start_requests() has " - f"been deprecated in favor of a new method, process_seeds(), " + f"been deprecated in favor of a new method, process_start(), " f"to support asynchronous code execution. " f"process_start_requests() will stop being called in a future " f"version of Scrapy. If you use Scrapy VERSION or higher " f"only, replace process_start_requests() with " - f"process_seeds(); note that process_seeds() is a coroutine " + f"process_start(); note that process_start() is a coroutine " f"(async def). If you need to maintain compatibility with " f"lower Scrapy versions, when defining " f"process_start_requests() in a spider middleware class, " - f"define process_seeds() as well. See the release notes of " + f"define process_start() as well. See the release notes of " f"Scrapy VERSION for details: " f"https://docs.scrapy.org/en/VERSION/news.html", ScrapyDeprecationWarning, @@ -104,8 +123,8 @@ class SpiderMiddlewareManager(MiddlewareManager): self.methods["process_start_requests"].appendleft( mw.process_start_requests ) - elif hasattr(mw, "process_seeds"): - self.methods["process_seeds"].appendleft(mw.process_seeds) + elif hasattr(mw, "process_start"): + self.methods["process_start"].appendleft(mw.process_start) process_spider_output = self._get_async_method_pair(mw, "process_spider_output") self.methods["process_spider_output"].appendleft(process_spider_output) process_spider_exception = getattr(mw, "process_spider_exception", None) @@ -117,7 +136,7 @@ class SpiderMiddlewareManager(MiddlewareManager): response: Response, request: Request, spider: Spider, - ) -> Iterable[_T] | AsyncIterable[_T]: + ) -> Iterable[_T] | AsyncIterator[_T]: for method in self.methods["process_spider_input"]: method = cast(Callable, method) try: @@ -138,10 +157,10 @@ class SpiderMiddlewareManager(MiddlewareManager): self, response: Response, spider: Spider, - iterable: Iterable[_T] | AsyncIterable[_T], + iterable: Iterable[_T] | AsyncIterator[_T], exception_processor_index: int, recover_to: MutableChain[_T] | MutableAsyncChain[_T], - ) -> Iterable[_T] | AsyncIterable[_T]: + ) -> Iterable[_T] | AsyncIterator[_T]: def process_sync(iterable: Iterable[_T]) -> Iterable[_T]: try: yield from iterable @@ -157,7 +176,7 @@ class SpiderMiddlewareManager(MiddlewareManager): assert isinstance(recover_to, MutableChain) recover_to.extend(exception_result) - async def process_async(iterable: AsyncIterable[_T]) -> AsyncIterable[_T]: + async def process_async(iterable: AsyncIterator[_T]) -> AsyncIterator[_T]: try: async for r in iterable: yield r @@ -173,7 +192,7 @@ class SpiderMiddlewareManager(MiddlewareManager): assert isinstance(recover_to, MutableAsyncChain) recover_to.extend(exception_result) - if isinstance(iterable, AsyncIterable): + if isinstance(iterable, AsyncIterator): return process_async(iterable) return process_sync(iterable) @@ -232,13 +251,13 @@ class SpiderMiddlewareManager(MiddlewareManager): self, response: Response, spider: Spider, - result: Iterable[_T] | AsyncIterable[_T], + result: Iterable[_T] | AsyncIterator[_T], start_index: int = 0, ) -> Generator[Deferred[Any], Any, MutableChain[_T] | MutableAsyncChain[_T]]: # items in this iterable do not need to go through the process_spider_output # chain, they went through it already from the process_spider_exception method recovered: MutableChain[_T] | MutableAsyncChain[_T] - last_result_is_async = isinstance(result, AsyncIterable) + last_result_is_async = isinstance(result, AsyncIterator) recovered = MutableAsyncChain() if last_result_is_async else MutableChain() # There are three cases for the middleware: def foo, async def foo, def foo + async def foo_async. @@ -265,7 +284,7 @@ class SpiderMiddlewareManager(MiddlewareManager): need_downgrade = True try: if need_upgrade: - # Iterable -> AsyncIterable + # Iterable -> AsyncIterator result = as_async_generator(result) elif need_downgrade: logger.warning( @@ -275,10 +294,10 @@ class SpiderMiddlewareManager(MiddlewareManager): f" https://docs.scrapy.org/en/latest/topics/coroutines.html#for-middleware-users" f" for more information." ) - assert isinstance(result, AsyncIterable) - # AsyncIterable -> Iterable + assert isinstance(result, AsyncIterator) + # AsyncIterator -> Iterable result = yield deferred_from_coro(collect_asyncgen(result)) - if isinstance(recovered, AsyncIterable): + if isinstance(recovered, AsyncIterator): recovered_collected = yield deferred_from_coro( collect_asyncgen(recovered) ) @@ -311,7 +330,7 @@ class SpiderMiddlewareManager(MiddlewareManager): f"{type(result)}" ) raise _InvalidOutput(msg) - last_result_is_async = isinstance(result, AsyncIterable) + last_result_is_async = isinstance(result, AsyncIterator) if last_result_is_async: return MutableAsyncChain(result, recovered) @@ -321,23 +340,23 @@ class SpiderMiddlewareManager(MiddlewareManager): self, response: Response, spider: Spider, - result: Iterable[_T] | AsyncIterable[_T], + result: Iterable[_T] | AsyncIterator[_T], ) -> MutableChain[_T] | MutableAsyncChain[_T]: recovered: MutableChain[_T] | MutableAsyncChain[_T] - if isinstance(result, AsyncIterable): + if isinstance(result, AsyncIterator): recovered = MutableAsyncChain() else: recovered = MutableChain() result = self._evaluate_iterable(response, spider, result, 0, recovered) result = await maybe_deferred_to_future( cast( - "Deferred[Iterable[_T] | AsyncIterable[_T]]", + "Deferred[Iterable[_T] | AsyncIterator[_T]]", self._process_spider_output(response, spider, result), ) ) - if isinstance(result, AsyncIterable): + if isinstance(result, AsyncIterator): return MutableAsyncChain(result, recovered) - if isinstance(recovered, AsyncIterable): + if isinstance(recovered, AsyncIterator): recovered_collected = await collect_asyncgen(recovered) recovered = MutableChain(recovered_collected) return MutableChain(result, recovered) @@ -350,7 +369,7 @@ class SpiderMiddlewareManager(MiddlewareManager): spider: Spider, ) -> Deferred[MutableChain[_T] | MutableAsyncChain[_T]]: async def process_callback_output( - result: Iterable[_T] | AsyncIterable[_T], + result: Iterable[_T] | AsyncIterator[_T], ) -> MutableChain[_T] | MutableAsyncChain[_T]: return await self._process_callback_output(response, spider, result) @@ -359,7 +378,7 @@ class SpiderMiddlewareManager(MiddlewareManager): ) -> Failure | MutableChain[_T] | MutableAsyncChain[_T]: return self._process_spider_exception(response, spider, _failure) - dfd: Deferred[Iterable[_T] | AsyncIterable[_T]] = mustbe_deferred( + dfd: Deferred[Iterable[_T] | AsyncIterator[_T]] = mustbe_deferred( self._process_spider_input, scrape_func, response, request, spider ) dfd2: Deferred[MutableChain[_T] | MutableAsyncChain[_T]] = dfd.addCallback( @@ -368,41 +387,41 @@ class SpiderMiddlewareManager(MiddlewareManager): dfd2.addErrback(process_spider_exception) return dfd2 - @inlineCallbacks - def process_seeds( - self, spider: Spider - ) -> Generator[Deferred[Any], Any, AsyncIterator[Any] | None]: + @deferred_f_from_coro_f + async def process_start(self, spider: Spider) -> AsyncIterator[Any] | None: try: self._check_deprecated_start_requests_use(spider) except ValueError as exception: logger.error(exception) return None - seeds: AsyncIterator[Any] + start: AsyncIterator[Any] if self._use_start_requests: - sync_seeds = iter(spider.start_requests()) - sync_seeds = yield self._process_chain( - "process_start_requests", sync_seeds, spider + sync_start = iter(spider.start_requests()) + sync_start = await maybe_deferred_to_future( + self._process_chain("process_start_requests", sync_start, spider) ) - seeds = as_async_generator(sync_seeds) + start = as_async_generator(sync_start) else: error_found = False - for fn in (spider.yield_seeds, *self.methods["process_seeds"]): + for fn in (spider.start, *self.methods["process_start"]): if isasyncgenfunction(fn): continue logger.error( - f"{global_object_name(fn)} must be an async generator " - f"function, i.e. an async def function with yield " + f"{global_object_name(fn)} must be an asynchronous " + f"generator, i.e. an async def function with yield " f"statements." ) error_found = True if error_found: return None - seeds = yield self._process_chain("process_seeds", spider.yield_seeds()) - return seeds + start = await maybe_deferred_to_future( + self._process_chain("process_start", spider.start()) + ) + return start def _check_deprecated_start_requests_use(self, spider: Spider): start_requests_cls = None - yield_seeds_cls = None + start_cls = None spidercls = spider.__class__ mro = spidercls.__mro__ @@ -410,19 +429,19 @@ class SpiderMiddlewareManager(MiddlewareManager): cls_dict = cls.__dict__ if start_requests_cls is None and "start_requests" in cls_dict: start_requests_cls = cls - if yield_seeds_cls is None and "yield_seeds" in cls_dict: - yield_seeds_cls = cls - if start_requests_cls is not None and yield_seeds_cls is not None: + if start_cls is None and "start" in cls_dict: + start_cls = cls + if start_requests_cls is not None and start_cls is not None: break - # Spider defines both, start_requests and yield_seeds. + # Spider defines both, start_requests and start. assert start_requests_cls is not None - assert yield_seeds_cls is not None + assert start_cls is not None if ( start_requests_cls is not Spider - and yield_seeds_cls is not start_requests_cls - and mro.index(start_requests_cls) < mro.index(yield_seeds_cls) + and start_cls is not start_requests_cls + and mro.index(start_requests_cls) < mro.index(start_cls) ): src = global_object_name(start_requests_cls) if start_requests_cls is not spidercls: @@ -430,15 +449,15 @@ class SpiderMiddlewareManager(MiddlewareManager): warn( f"{src} defines the deprecated start_requests() method. " f"start_requests() has been deprecated in favor of a new " - f"method, yield_seeds(), to support asynchronous code " + f"method, start(), to support asynchronous code " f"execution. start_requests() will stop being called in a " f"future version of Scrapy. If you use Scrapy VERSION or " - f"higher only, replace start_requests() with yield_seeds(); " - f"note that yield_seeds() is a coroutine (async def). If you " + f"higher only, replace start_requests() with start(); " + f"note that start() is a coroutine (async def). If you " f"need to maintain compatibility with lower Scrapy versions, " f"when overriding start_requests() in a spider class, " - f"override yield_seeds() as well; you can use super() to " - f"reuse the inherited yield_seeds() implementation without " + f"override start() as well; you can use super() to " + f"reuse the inherited start() implementation without " f"copy-pasting. See the release notes of Scrapy VERSION for " f"details: https://docs.scrapy.org/en/VERSION/news.html", ScrapyDeprecationWarning, @@ -446,26 +465,26 @@ class SpiderMiddlewareManager(MiddlewareManager): if ( self._use_start_requests - and yield_seeds_cls is not Spider - and start_requests_cls is not yield_seeds_cls - and mro.index(yield_seeds_cls) < mro.index(start_requests_cls) + and start_cls is not Spider + and start_requests_cls is not start_cls + and mro.index(start_cls) < mro.index(start_requests_cls) ): - src = global_object_name(yield_seeds_cls) - if yield_seeds_cls is not spidercls: + src = global_object_name(start_cls) + if start_cls is not spidercls: src += f" (inherited by {global_object_name(spidercls)})" raise ValueError( f"{src} does not define the deprecated start_requests() " f"method. However, one or more of your enabled spider " f"middlewares (reported in an earlier deprecation warning) " f"define the process_start_requests() method, and not the " - f"process_seeds() method, making them only compatible with " + f"process_start() method, making them only compatible with " f"(deprecated) spiders that define the start_requests() " f"method. To solve this issue, disable the offending spider " f"middlewares, upgrade them as described in that earlier " f"deprecation warning, or make your spider compatible with " f"deprecated spider middlewares (and earlier Scrapy versions) " f"by defining a sync start_requests() method that works " - f"similarly to its existing yield_seeds() method. See the " + f"similarly to its existing start() method. See the " f"release notes of Scrapy VERSION for details: " f"https://docs.scrapy.org/en/VERSION/news.html" ) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 88d618085..749096db5 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -136,6 +136,9 @@ class Crawler: "Overridden settings:\n%(settings)s", {"settings": pprint.pformat(d)} ) + # Cannot use @deferred_f_from_coro_f because that relies on the reactor + # being installed already, which is done within _apply_settings(), inside + # this method. @inlineCallbacks def crawl(self, *args: Any, **kwargs: Any) -> Generator[Deferred[Any], Any, None]: if self.crawling: @@ -152,7 +155,7 @@ class Crawler: self._update_root_log_handler() self.engine = self._create_engine() yield self.engine.open_spider(self.spider) - yield maybeDeferred(self.engine.start) + yield self.engine.start() except Exception: self.crawling = False if self.engine is not None: diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 62b9697f1..2b8d0ab84 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -131,6 +131,8 @@ class Request(object_ref): if not isinstance(priority, int): raise TypeError(f"Request priority not an integer: {priority!r}") + #: Default: ``0`` + #: #: Value that the :ref:`scheduler ` may use for #: request prioritization. #: @@ -199,7 +201,7 @@ class Request(object_ref): #: #: When defining the start URLs of a spider through #: :attr:`~scrapy.Spider.start_urls`, this attribute is enabled by - #: default. See :meth:`~scrapy.Spider.yield_seeds`. + #: default. See :meth:`~scrapy.Spider.start`. self.dont_filter: bool = dont_filter self._meta: dict[str, Any] | None = dict(meta) if meta else None diff --git a/scrapy/logformatter.py b/scrapy/logformatter.py index 6315b7adc..4f08918ae 100644 --- a/scrapy/logformatter.py +++ b/scrapy/logformatter.py @@ -98,7 +98,7 @@ class LogFormatter: """Logs a message when an item is scraped by a spider.""" src: Any if response is None: - src = f"{global_object_name(spider.__class__)}.yield_seeds" + src = f"{global_object_name(spider.__class__)}.start" elif isinstance(response, Failure): src = response.getErrorMessage() else: diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 886154bfa..680fded7a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -17,8 +17,6 @@ import sys from importlib import import_module from pathlib import Path -from scrapy import SeedingPolicy - ADDONS = {} AJAXCRAWL_ENABLED = False @@ -310,8 +308,6 @@ SCHEDULER_PRIORITY_QUEUE = "scrapy.pqueues.ScrapyPriorityQueue" SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5000000 -SEEDING_POLICY = SeedingPolicy.greedy - SPIDER_LOADER_CLASS = "scrapy.spiderloader.SpiderLoader" SPIDER_LOADER_WARN_ONLY = False @@ -355,3 +351,5 @@ SPIDER_CONTRACTS_BASE = { "scrapy.contracts.default.ReturnsContract": 2, "scrapy.contracts.default.ScrapesContract": 3, } + +WARN_ON_GENERATOR_RETURN_VALUE = True diff --git a/scrapy/shell.py b/scrapy/shell.py index 5e5e57a9a..bb39eccc3 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -24,6 +24,7 @@ from scrapy.spiders import Spider from scrapy.utils.conf import get_config from scrapy.utils.console import DEFAULT_PYTHON_SHELLS, start_python_console from scrapy.utils.datatypes import SequenceExclude +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.misc import load_object from scrapy.utils.reactor import is_asyncio_reactor_installed, set_asyncio_event_loop from scrapy.utils.response import open_in_browser @@ -102,25 +103,33 @@ class Shell: # set the asyncio event loop for the current thread event_loop_path = self.crawler.settings["ASYNCIO_EVENT_LOOP"] set_asyncio_event_loop(event_loop_path) - spider = self._open_spider(request, spider) + + def crawl_request(_): + assert self.crawler.engine is not None + self.crawler.engine.crawl(request) + + d2 = self._open_spider(request, spider) + d2.addCallback(crawl_request) + d = _request_deferred(request) d.addCallback(lambda x: (x, spider)) - assert self.crawler.engine - self.crawler.engine.crawl(request) return d - def _open_spider(self, request: Request, spider: Spider | None) -> Spider: + @deferred_f_from_coro_f + async def _open_spider(self, request: Request, spider: Spider | None) -> None: if self.spider: - return self.spider + return if spider is None: spider = self.crawler.spider or self.crawler._create_spider() self.crawler.spider = spider assert self.crawler.engine - self.crawler.engine.open_spider(spider, close_if_idle=False) + await maybe_deferred_to_future( + self.crawler.engine.open_spider(spider, close_if_idle=False) + ) + self.crawler.engine._start_request_processing() self.spider = spider - return spider def fetch( self, diff --git a/scrapy/signalmanager.py b/scrapy/signalmanager.py index e106418d6..f8c50b5e3 100644 --- a/scrapy/signalmanager.py +++ b/scrapy/signalmanager.py @@ -1,13 +1,12 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from typing import Any from pydispatch import dispatcher +from twisted.internet.defer import Deferred from scrapy.utils import signal as _signal - -if TYPE_CHECKING: - from twisted.internet.defer import Deferred +from scrapy.utils.defer import maybe_deferred_to_future class SignalManager: @@ -75,3 +74,17 @@ class SignalManager: """ kwargs.setdefault("sender", self.sender) _signal.disconnect_all(signal, **kwargs) + + async def wait_for(self, signal): + """Await the next *signal*. + + See :ref:`start-requests-lazy` for an example. + """ + d = Deferred() + + def handle(): + self.disconnect(handle, signal) + d.callback(None) + + self.connect(handle, signal) + await maybe_deferred_to_future(d) diff --git a/scrapy/signals.py b/scrapy/signals.py index 8ef0f34f0..bdeec1ba0 100644 --- a/scrapy/signals.py +++ b/scrapy/signals.py @@ -7,6 +7,7 @@ signals here without documenting them there. engine_started = object() engine_stopped = object() +scheduler_empty = object() spider_opened = object() spider_idle = object() spider_closed = object() diff --git a/scrapy/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 3164c1c03..5760411c0 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -12,7 +12,7 @@ from typing import TYPE_CHECKING, Any from scrapy.http import Request, Response if TYPE_CHECKING: - from collections.abc import AsyncIterable, Iterable + from collections.abc import AsyncIterator, Iterable # typing.Self requires Python 3.11 from typing_extensions import Self @@ -54,8 +54,8 @@ class DepthMiddleware: return (r for r in result if self._filter(r, response, spider)) async def process_spider_output_async( - self, response: Response, result: AsyncIterable[Any], spider: Spider - ) -> AsyncIterable[Any]: + self, response: Response, result: AsyncIterator[Any], spider: Spider + ) -> AsyncIterator[Any]: self._init_depth(response, spider) async for r in result: if self._filter(r, response, spider): diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index 646beb911..060be1975 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -23,7 +23,7 @@ warnings.warn( ) if TYPE_CHECKING: - from collections.abc import AsyncIterable, Iterable + from collections.abc import AsyncIterator, Iterable # typing.Self requires Python 3.11 from typing_extensions import Self @@ -52,8 +52,8 @@ class OffsiteMiddleware: return (r for r in result if self._filter(r, spider)) async def process_spider_output_async( - self, response: Response, result: AsyncIterable[Any], spider: Spider - ) -> AsyncIterable[Any]: + self, response: Response, result: AsyncIterator[Any], spider: Spider + ) -> AsyncIterator[Any]: async for r in result: if self._filter(r, spider): yield r diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index a3a1e5b92..c824579b0 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -19,7 +19,7 @@ from scrapy.utils.python import to_unicode from scrapy.utils.url import strip_url if TYPE_CHECKING: - from collections.abc import AsyncIterable, Iterable + from collections.abc import AsyncIterator, Iterable # typing.Self requires Python 3.11 from typing_extensions import Self @@ -376,8 +376,8 @@ class RefererMiddleware: return (self._set_referer(r, response) for r in result) async def process_spider_output_async( - self, response: Response, result: AsyncIterable[Any], spider: Spider - ) -> AsyncIterable[Any]: + self, response: Response, result: AsyncIterator[Any], spider: Spider + ) -> AsyncIterator[Any]: async for r in result: yield self._set_referer(r, response) diff --git a/scrapy/spidermiddlewares/urllength.py b/scrapy/spidermiddlewares/urllength.py index a1cd1bb7c..9259fea3c 100644 --- a/scrapy/spidermiddlewares/urllength.py +++ b/scrapy/spidermiddlewares/urllength.py @@ -14,7 +14,7 @@ from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request, Response if TYPE_CHECKING: - from collections.abc import AsyncIterable, Iterable + from collections.abc import AsyncIterator, Iterable # typing.Self requires Python 3.11 from typing_extensions import Self @@ -57,8 +57,8 @@ class UrlLengthMiddleware: return (r for r in result if self._filter(r, spider)) async def process_spider_output_async( - self, response: Response, result: AsyncIterable[Any], spider: Spider - ) -> AsyncIterable[Any]: + self, response: Response, result: AsyncIterator[Any], spider: Spider + ) -> AsyncIterator[Any]: async for r in result: if self._filter(r, spider): yield r diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 270c1d8ba..36d64b0a1 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -7,9 +7,11 @@ See documentation in docs/topics/spiders.rst from __future__ import annotations import logging +import warnings from typing import TYPE_CHECKING, Any, cast from scrapy import signals +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.utils.trackref import object_ref from scrapy.utils.url import url_is_from_spider @@ -31,7 +33,7 @@ if TYPE_CHECKING: class Spider(object_ref): """Base class that any spider must subclass. - It provides a default :meth:`yield_seeds` implementation that sends + It provides a default :meth:`start` implementation that sends requests based on the :attr:`start_urls` class attribute and calls the :meth:`parse` method for each response. """ @@ -39,7 +41,7 @@ class Spider(object_ref): name: str custom_settings: dict[_SettingsKeyT, Any] | None = None - #: Seed URLs. See :meth:`yield_seeds`. + #: Start URLs. See :meth:`start`. start_urls: list[str] def __init__(self, name: str | None = None, **kwargs: Any): @@ -78,7 +80,7 @@ class Spider(object_ref): self.settings: BaseSettings = crawler.settings crawler.signals.connect(self.close, signals.spider_closed) - async def yield_seeds(self) -> AsyncIterator[Any]: + async def start(self) -> AsyncIterator[Any]: """Yield the initial :class:`~scrapy.Request` objects to send. .. versionadded:: VERSION @@ -93,7 +95,7 @@ class Spider(object_ref): class MySpider(Spider): name = "myspider" - async def yield_seeds(self): + async def start(self): yield Request("https://toscrape.com/") The default implementation reads URLs from :attr:`start_urls` and @@ -102,7 +104,7 @@ class Spider(object_ref): .. code-block:: python - async def yield_seeds(self): + async def start(self): for url in self.start_urls: yield Request(url, dont_filter=True) @@ -110,34 +112,21 @@ class Spider(object_ref): .. code-block:: python - async def yield_seeds(self): + async def start(self): yield {"foo": "bar"} - Use :setting:`SEEDING_POLICY` to set how :meth:`yield_seeds` is - iterated by default. It is also - possible to yield a :class:`~scrapy.SeedingPolicy` enum or a matching - string to change the active seeding policy, for example: - - .. code-block:: python - - async def yield_seeds(self): - yield "front_load" - yield Request("https://a.example") - yield Request("https://b.example") - yield self.crawler.settings["SEEDING_POLICY"] - yield Request("https://c.example") - It is also possible to raise :exc:`~scrapy.exceptions.CloseSpider`, for - example to customize the close reason when there are no seeds to yield: + example to customize the close reason when there are no start requests + to yield: .. code-block:: python - async def yield_seeds(self): - seeds = await queue_service_client.get_seed_batch() - if not seeds: - raise CloseSpider("no_seeds") - for seed in seeds: - yield seed + async def start(self): + requests = await queue_service_client.get_request_batch() + if not requests: + raise CloseSpider("no_start_requests") + for request in requests: + yield request To write spiders that work on Scrapy versions lower than VERSION, define also a synchronous ``start_requests()`` method that returns an @@ -147,11 +136,27 @@ class Spider(object_ref): def start_requests(self): yield Request("https://toscrape.com/") + + .. seealso:: :ref:`start-requests` """ - for seed in self.start_requests(): - yield seed + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", category=ScrapyDeprecationWarning, module=r"^scrapy\.spiders$" + ) + for item_or_request in self.start_requests(): + yield item_or_request def start_requests(self) -> Iterable[Any]: + warnings.warn( + ( + "The Spider.start_requests() method is deprecated, use " + "Spider.start() instead. If you are calling " + "super().start_requests() from a Spider.start() override, " + "iterate super().start() instead." + ), + ScrapyDeprecationWarning, + stacklevel=2, + ) if not self.start_urls and hasattr(self, "start_url"): raise AttributeError( "Crawling could not start: 'start_urls' not found " diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index 087049425..171d8479c 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -8,7 +8,7 @@ See documentation in docs/topics/spiders.rst from __future__ import annotations import copy -from collections.abc import AsyncIterable, Awaitable, Callable +from collections.abc import AsyncIterator, Awaitable, Callable from typing import TYPE_CHECKING, Any, Optional, TypeVar, cast from twisted.python.failure import Failure @@ -156,10 +156,10 @@ class CrawlSpider(Spider): callback: CallbackT | None, cb_kwargs: dict[str, Any], follow: bool = True, - ) -> AsyncIterable[Any]: + ) -> AsyncIterator[Any]: if callback: cb_res = callback(response, **cb_kwargs) or () - if isinstance(cb_res, AsyncIterable): + if isinstance(cb_res, AsyncIterator): cb_res = await collect_asyncgen(cb_res) elif isinstance(cb_res, Awaitable): cb_res = await cb_res diff --git a/scrapy/spiders/init.py b/scrapy/spiders/init.py index 5c84ae5fe..e5548b9fa 100644 --- a/scrapy/spiders/init.py +++ b/scrapy/spiders/init.py @@ -29,9 +29,13 @@ class InitSpider(Spider): stacklevel=2, ) - async def yield_seeds(self) -> AsyncIterator[Any]: - for seed in self.start_requests(): - yield seed + async def start(self) -> AsyncIterator[Any]: + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", category=ScrapyDeprecationWarning, module=r"^scrapy\.spiders$" + ) + for item_or_request in self.start_requests(): + yield item_or_request def start_requests(self) -> Iterable[Request]: self._postinit_reqs: Iterable[Request] = super().start_requests() diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 1be23421c..2813a32a0 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -53,9 +53,9 @@ class SitemapSpider(Spider): self._cbs.append((regex(r), c)) self._follow: list[re.Pattern[str]] = [regex(x) for x in self.sitemap_follow] - async def yield_seeds(self) -> AsyncIterator[Any]: - for seed in self.start_requests(): - yield seed + async def start(self) -> AsyncIterator[Any]: + for item_or_request in self.start_requests(): + yield item_or_request def start_requests(self) -> Iterable[Request]: for url in self.sitemap_urls: diff --git a/scrapy/templates/project/module/middlewares.py.tmpl b/scrapy/templates/project/module/middlewares.py.tmpl index 8b8ab927d..3f0239832 100644 --- a/scrapy/templates/project/module/middlewares.py.tmpl +++ b/scrapy/templates/project/module/middlewares.py.tmpl @@ -43,11 +43,11 @@ class ${ProjectName}SpiderMiddleware: # Should return either None or an iterable of Request or item objects. pass - async def process_seeds(self, seeds): - # Called with the seeds from the spider yield_seeds() method or with - # the output of the maching method of an earlier spider middleware. - async for seed in seeds: - yield seed + async def process_start(self, start): + # Called with an async iterator over the spider start() method or the + # maching method of an earlier spider middleware. + async for item_or_request in start: + yield item_or_request def spider_opened(self, spider): spider.logger.info("Spider opened: %s" % spider.name) diff --git a/scrapy/utils/asyncgen.py b/scrapy/utils/asyncgen.py index 237bd8331..6d96a41f5 100644 --- a/scrapy/utils/asyncgen.py +++ b/scrapy/utils/asyncgen.py @@ -1,20 +1,20 @@ from __future__ import annotations -from collections.abc import AsyncGenerator, AsyncIterable, Iterable +from collections.abc import AsyncGenerator, AsyncIterator, Iterable from typing import TypeVar _T = TypeVar("_T") -async def collect_asyncgen(result: AsyncIterable[_T]) -> list[_T]: +async def collect_asyncgen(result: AsyncIterator[_T]) -> list[_T]: return [x async for x in result] async def as_async_generator( - it: Iterable[_T] | AsyncIterable[_T], + it: Iterable[_T] | AsyncIterator[_T], ) -> AsyncGenerator[_T]: """Wraps an iterable (sync or async) into an async generator.""" - if isinstance(it, AsyncIterable): + if isinstance(it, AsyncIterator): async for r in it: yield r else: diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 42ad28d8d..c39837716 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -22,7 +22,7 @@ from scrapy.exceptions import IgnoreRequest, ScrapyDeprecationWarning from scrapy.utils.reactor import _get_asyncio_event_loop, is_asyncio_reactor_installed if TYPE_CHECKING: - from collections.abc import AsyncIterable, AsyncIterator, Callable + from collections.abc import AsyncIterator, Callable from twisted.python.failure import Failure @@ -177,7 +177,7 @@ class _AsyncCooperatorAdapter(Iterator, Generic[_T]): def __init__( self, - aiterable: AsyncIterable[_T], + aiterable: AsyncIterator[_T], callable: Callable[Concatenate[_T, _P], Deferred[Any] | None], *callable_args: _P.args, **callable_kwargs: _P.kwargs, @@ -234,7 +234,7 @@ class _AsyncCooperatorAdapter(Iterator, Generic[_T]): def parallel_async( - async_iterable: AsyncIterable[_T], + async_iterable: AsyncIterator[_T], count: int, callable: Callable[Concatenate[_T, _P], Deferred[Any] | None], *args: _P.args, @@ -332,11 +332,11 @@ def iter_errback( async def aiter_errback( - aiterable: AsyncIterable[_T], + aiterable: AsyncIterator[_T], errback: Callable[Concatenate[Failure, _P], Any], *a: _P.args, **kw: _P.kwargs, -) -> AsyncIterable[_T]: +) -> AsyncIterator[_T]: """Wraps an async iterable calling an errback if an error is caught while iterating it. Similar to scrapy.utils.defer.iter_errback() """ diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index d319e7950..b7b436260 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -286,6 +286,8 @@ def warn_on_generator_with_return_value( Logs a warning if a callable is a generator function and includes a 'return' statement with a value different than None """ + if not spider.settings.getbool("WARN_ON_GENERATOR_RETURN_VALUE"): + return try: if is_generator_with_return_value(callable): warnings.warn( diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 2e6869779..1d9e34f35 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -10,7 +10,7 @@ import re import sys import warnings import weakref -from collections.abc import AsyncIterable, Iterable, Mapping +from collections.abc import AsyncIterator, Iterable, Mapping from functools import partial, wraps from itertools import chain from typing import TYPE_CHECKING, Any, TypeVar, overload @@ -19,11 +19,11 @@ from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.asyncgen import as_async_generator if TYPE_CHECKING: - from collections.abc import AsyncIterator, Callable, Iterator + from collections.abc import Callable, Iterator from re import Pattern - # typing.Concatenate and typing.ParamSpec require Python 3.10 - from typing_extensions import Concatenate, ParamSpec + # typing.Concatenate, typing.ParamSpec and typing.Self require Python 3.10 + from typing_extensions import Concatenate, ParamSpec, Self _P = ParamSpec("_P") @@ -369,25 +369,25 @@ class MutableChain(Iterable[_T]): async def _async_chain( - *iterables: Iterable[_T] | AsyncIterable[_T], + *iterables: Iterable[_T] | AsyncIterator[_T], ) -> AsyncIterator[_T]: for it in iterables: async for o in as_async_generator(it): yield o -class MutableAsyncChain(AsyncIterable[_T]): +class MutableAsyncChain(AsyncIterator[_T]): """ Similar to MutableChain but for async iterables """ - def __init__(self, *args: Iterable[_T] | AsyncIterable[_T]): + def __init__(self, *args: Iterable[_T] | AsyncIterator[_T]): self.data: AsyncIterator[_T] = _async_chain(*args) - def extend(self, *iterables: Iterable[_T] | AsyncIterable[_T]) -> None: + def extend(self, *iterables: Iterable[_T] | AsyncIterator[_T]) -> None: self.data = _async_chain(self.data, _async_chain(*iterables)) - def __aiter__(self) -> AsyncIterator[_T]: + def __aiter__(self) -> Self: return self async def __anext__(self) -> _T: diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py index 099c81f0e..9c2754394 100644 --- a/scrapy/utils/reactor.py +++ b/scrapy/utils/reactor.py @@ -7,6 +7,7 @@ from typing import TYPE_CHECKING, Any, Generic, TypeVar from warnings import catch_warnings, filterwarnings from twisted.internet import asyncioreactor, error +from twisted.internet.defer import Deferred from scrapy.utils.misc import load_object @@ -54,6 +55,7 @@ class CallLaterOnce(Generic[_T]): self._a: tuple[Any, ...] = a self._kw: dict[str, Any] = kw self._call: DelayedCall | None = None + self._deferreds: list[Deferred] = [] def schedule(self, delay: float = 0) -> None: from twisted.internet import reactor @@ -66,8 +68,23 @@ class CallLaterOnce(Generic[_T]): self._call.cancel() def __call__(self) -> _T: + from twisted.internet import reactor + self._call = None - return self._func(*self._a, **self._kw) + result = self._func(*self._a, **self._kw) + + for d in self._deferreds: + reactor.callLater(0, d.callback, None) + self._deferreds = [] + + return result + + async def wait(self): + from scrapy.utils.defer import maybe_deferred_to_future + + d = Deferred() + self._deferreds.append(d) + await maybe_deferred_to_future(d) def set_asyncio_event_loop_policy() -> None: @@ -114,8 +131,10 @@ def set_asyncio_event_loop(event_loop_path: str | None) -> AbstractEventLoop: """Sets and returns the event loop with specified import path.""" if event_loop_path is not None: event_loop_class: type[AbstractEventLoop] = load_object(event_loop_path) - event_loop = event_loop_class() - asyncio.set_event_loop(event_loop) + event_loop = _get_asyncio_event_loop() + if not isinstance(event_loop, event_loop_class): + event_loop = event_loop_class() + asyncio.set_event_loop(event_loop) else: try: with catch_warnings(): diff --git a/sep/sep-018.rst b/sep/sep-018.rst index e6d601fe1..29b1f860e 100644 --- a/sep/sep-018.rst +++ b/sep/sep-018.rst @@ -619,7 +619,7 @@ Resolved: ``manager.scraper.process_request()`` instead of ``manager.engine.crawl()`` - should we support adding additional start requests from a spider middleware? - - Yes - there is a spider middleware method (``start_requests``) for that + - Yes - there is a spider middleware method (``start_requests()``) for that - should ``process_response()`` receive a ``request`` argument with the ``request`` that originated it?. ``response.request`` is the latest request, not the original one (think of redirections), but it does carry the ``meta`` diff --git a/tests/CrawlerProcess/args_settings.py b/tests/CrawlerProcess/args_settings.py index 6076211ec..c8a3d0a5b 100644 --- a/tests/CrawlerProcess/args_settings.py +++ b/tests/CrawlerProcess/args_settings.py @@ -13,7 +13,7 @@ class NoRequestsSpider(scrapy.Spider): spider.settings.set("FOO", kwargs.get("foo")) return spider - async def yield_seeds(self): + async def start(self): self.logger.info(f"The value of FOO is {self.settings.getint('FOO')}") return yield diff --git a/tests/CrawlerProcess/asyncio_custom_loop.py b/tests/CrawlerProcess/asyncio_custom_loop.py index 13c6fff85..bd78a0de7 100644 --- a/tests/CrawlerProcess/asyncio_custom_loop.py +++ b/tests/CrawlerProcess/asyncio_custom_loop.py @@ -5,7 +5,7 @@ from scrapy.crawler import CrawlerProcess class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/asyncio_enabled_no_reactor.py b/tests/CrawlerProcess/asyncio_enabled_no_reactor.py index 950bdb006..6bb6fb3c6 100644 --- a/tests/CrawlerProcess/asyncio_enabled_no_reactor.py +++ b/tests/CrawlerProcess/asyncio_enabled_no_reactor.py @@ -12,7 +12,7 @@ class ReactorCheckExtension: class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/asyncio_enabled_reactor.py b/tests/CrawlerProcess/asyncio_enabled_reactor.py index a80a1180a..f3dab12fe 100644 --- a/tests/CrawlerProcess/asyncio_enabled_reactor.py +++ b/tests/CrawlerProcess/asyncio_enabled_reactor.py @@ -38,7 +38,7 @@ class ReactorCheckExtension: class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/asyncio_enabled_reactor_different_loop.py b/tests/CrawlerProcess/asyncio_enabled_reactor_different_loop.py index 59feced94..d8c467f40 100644 --- a/tests/CrawlerProcess/asyncio_enabled_reactor_different_loop.py +++ b/tests/CrawlerProcess/asyncio_enabled_reactor_different_loop.py @@ -15,7 +15,7 @@ from scrapy.crawler import CrawlerProcess # noqa: E402 class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/asyncio_enabled_reactor_same_loop.py b/tests/CrawlerProcess/asyncio_enabled_reactor_same_loop.py index f297fd0d7..e7d3ca9cc 100644 --- a/tests/CrawlerProcess/asyncio_enabled_reactor_same_loop.py +++ b/tests/CrawlerProcess/asyncio_enabled_reactor_same_loop.py @@ -16,7 +16,7 @@ from scrapy.crawler import CrawlerProcess # noqa: E402 class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/caching_hostname_resolver.py b/tests/CrawlerProcess/caching_hostname_resolver.py index 26343da6a..53d427061 100644 --- a/tests/CrawlerProcess/caching_hostname_resolver.py +++ b/tests/CrawlerProcess/caching_hostname_resolver.py @@ -11,7 +11,7 @@ class CachingHostnameResolverSpider(scrapy.Spider): name = "caching_hostname_resolver_spider" - async def yield_seeds(self): + async def start(self): yield scrapy.Request(self.url) def parse(self, response): diff --git a/tests/CrawlerProcess/multi.py b/tests/CrawlerProcess/multi.py index 65f5e033f..0058896b5 100644 --- a/tests/CrawlerProcess/multi.py +++ b/tests/CrawlerProcess/multi.py @@ -5,7 +5,7 @@ from scrapy.crawler import CrawlerProcess class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/reactor_default.py b/tests/CrawlerProcess/reactor_default.py index a221764d2..8f59c035c 100644 --- a/tests/CrawlerProcess/reactor_default.py +++ b/tests/CrawlerProcess/reactor_default.py @@ -8,7 +8,7 @@ from scrapy.crawler import CrawlerProcess class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/reactor_default_twisted_reactor_select.py b/tests/CrawlerProcess/reactor_default_twisted_reactor_select.py index a0aff999b..9901dd634 100644 --- a/tests/CrawlerProcess/reactor_default_twisted_reactor_select.py +++ b/tests/CrawlerProcess/reactor_default_twisted_reactor_select.py @@ -8,7 +8,7 @@ from scrapy.crawler import CrawlerProcess class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/reactor_select.py b/tests/CrawlerProcess/reactor_select.py index 6ac1043b5..53941568a 100644 --- a/tests/CrawlerProcess/reactor_select.py +++ b/tests/CrawlerProcess/reactor_select.py @@ -10,7 +10,7 @@ selectreactor.install() class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/reactor_select_subclass_twisted_reactor_select.py b/tests/CrawlerProcess/reactor_select_subclass_twisted_reactor_select.py index f7352af57..5739d77ae 100644 --- a/tests/CrawlerProcess/reactor_select_subclass_twisted_reactor_select.py +++ b/tests/CrawlerProcess/reactor_select_subclass_twisted_reactor_select.py @@ -17,7 +17,7 @@ installReactor(reactor) class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/reactor_select_twisted_reactor_select.py b/tests/CrawlerProcess/reactor_select_twisted_reactor_select.py index 1071e453d..c488f7526 100644 --- a/tests/CrawlerProcess/reactor_select_twisted_reactor_select.py +++ b/tests/CrawlerProcess/reactor_select_twisted_reactor_select.py @@ -9,7 +9,7 @@ selectreactor.install() class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerProcess/simple.py b/tests/CrawlerProcess/simple.py index 3773092b0..9e4ad70d9 100644 --- a/tests/CrawlerProcess/simple.py +++ b/tests/CrawlerProcess/simple.py @@ -5,7 +5,7 @@ from scrapy.crawler import CrawlerProcess class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerRunner/change_reactor.py b/tests/CrawlerRunner/change_reactor.py index bdc217fde..6c0102241 100644 --- a/tests/CrawlerRunner/change_reactor.py +++ b/tests/CrawlerRunner/change_reactor.py @@ -10,7 +10,7 @@ class NoRequestsSpider(Spider): "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", } - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/CrawlerRunner/ip_address.py b/tests/CrawlerRunner/ip_address.py index 892fab731..5e2184afb 100644 --- a/tests/CrawlerRunner/ip_address.py +++ b/tests/CrawlerRunner/ip_address.py @@ -32,7 +32,7 @@ def createResolver(servers=None, resolvconf=None, hosts=None): class LocalhostSpider(Spider): name = "localhost_spider" - async def yield_seeds(self): + async def start(self): yield Request(self.url) def parse(self, response): diff --git a/tests/__init__.py b/tests/__init__.py index cd52ade58..ccfabb0da 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -8,6 +8,9 @@ import os import socket from pathlib import Path +from twisted import version as TWISTED_VERSION +from twisted.python.versions import Version + # ignore system-wide proxies for tests # which would send requests to a totally unsuspecting server # (e.g. because urllib does not fully understand the proxy spec) @@ -30,3 +33,6 @@ except socket.gaierror: def get_testdata(*paths: str) -> bytes: """Return test data""" return Path(tests_datadir, *paths).read_bytes() + + +TWISTED_KEEPS_TRACEBACKS = TWISTED_VERSION >= Version("twisted", 24, 10, 0) diff --git a/tests/spiders.py b/tests/spiders.py index be3580e5a..c47f2bd2b 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -68,7 +68,7 @@ class DelaySpider(MetaSpider): self.b = b self.t1 = self.t2 = self.t2_err = 0 - async def yield_seeds(self): + async def start(self): self.t1 = time.time() url = self.mockserver.url(f"/delay?n={self.n}&b={self.b}") yield Request(url, callback=self.parse, errback=self.errback) @@ -105,7 +105,7 @@ class LogSpider(MetaSpider): class SlowSpider(DelaySpider): name = "slow" - async def yield_seeds(self): + async def start(self): # 1st response is fast url = self.mockserver.url("/delay?n=0&b=0") yield Request(url, callback=self.parse, errback=self.errback) @@ -255,7 +255,7 @@ class AsyncDefAsyncioGenComplexSpider(SimpleSpider): callback=cb, ) - async def yield_seeds(self): + async def start(self): for i in range(1, self.initial_reqs + 1): yield self._get_req(i) @@ -319,13 +319,39 @@ class ErrorSpider(FollowAllSpider): self.raise_exception() -class YieldSeedsItemSpider(FollowAllSpider): - async def yield_seeds(self): +class BrokenStartSpider(FollowAllSpider): + fail_before_yield = False + fail_yielding = False + + def __init__(self, *a, **kw): + super().__init__(*a, **kw) + self.seedsseen = [] + + async def start(self): + if self.fail_before_yield: + 1 / 0 + + for s in range(100): + qargs = {"total": 10, "seed": s} + url = self.mockserver.url(f"/follow?{urlencode(qargs, doseq=True)}") + yield Request(url, meta={"seed": s}) + if self.fail_yielding: + 2 / 0 + + assert self.seedsseen, "All seeds consumed before any download happened" + + def parse(self, response): + self.seedsseen.append(response.meta.get("seed")) + yield from super().parse(response) + + +class StartItemSpider(FollowAllSpider): + async def start(self): yield {"name": "test item"} -class YieldSeedsGoodAndBadOutput(FollowAllSpider): - async def yield_seeds(self): +class StartGoodAndBadOutput(FollowAllSpider): + async def start(self): yield {"a": "a"} yield Request("data:,a") yield "data:,b" @@ -337,7 +363,7 @@ class SingleRequestSpider(MetaSpider): callback_func = None errback_func = None - async def yield_seeds(self): + async def start(self): if isinstance(self.seed, Request): yield self.seed.replace(callback=self.parse, errback=self.on_error) else: @@ -358,13 +384,13 @@ class SingleRequestSpider(MetaSpider): return None -class DuplicateYieldSeedsSpider(MockServerSpider): +class DuplicateStartSpider(MockServerSpider): dont_filter = True name = "duplicatestartrequests" distinct_urls = 2 dupe_factor = 3 - async def yield_seeds(self): + async def start(self): for i in range(self.distinct_urls): for j in range(self.dupe_factor): url = self.mockserver.url(f"/echo?headers=1&body=test{i}") @@ -389,7 +415,7 @@ class CrawlSpiderWithParseMethod(MockServerSpider, CrawlSpider): } rules = (Rule(LinkExtractor(), callback="parse", follow=True),) - async def yield_seeds(self): + async def start(self): test_body = b""" Page title<title></head> @@ -443,7 +469,7 @@ class CrawlSpiderWithErrback(CrawlSpiderWithParseMethod): name = "crawl_spider_with_errback" rules = (Rule(LinkExtractor(), callback="parse", errback="errback", follow=True),) - async def yield_seeds(self): + async def start(self): test_body = b""" <html> <head><title>Page title<title></head> @@ -488,7 +514,7 @@ class BytesReceivedCallbackSpider(MetaSpider): crawler.signals.connect(spider.bytes_received, signals.bytes_received) return spider - async def yield_seeds(self): + async def start(self): body = b"a" * self.full_response_length url = self.mockserver.url("/alpayload") yield Request(url, method="POST", body=body, errback=self.errback) @@ -517,7 +543,7 @@ class HeadersReceivedCallbackSpider(MetaSpider): crawler.signals.connect(spider.headers_received, signals.headers_received) return spider - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status"), errback=self.errback) def parse(self, response): diff --git a/tests/test_closespider.py b/tests/test_closespider.py index d4551904f..e9b76a5c6 100644 --- a/tests/test_closespider.py +++ b/tests/test_closespider.py @@ -91,6 +91,7 @@ class TestCloseSpider(TestCase): assert reason == "closespider_errorcount" key = f"spider_exceptions/{crawler.spider.exception_cls.__name__}" errorcount = crawler.stats.get_value(key) + assert crawler.stats.get_value("spider_exceptions/count") >= close_on assert errorcount >= close_on @defer.inlineCallbacks @@ -114,11 +115,11 @@ class TestCloseSpider(TestCase): assert total_seconds >= timeout @deferred_f_from_coro_f - async def test_yield_seeds(self): + async def test_spider_start(self): class TestSpider(Spider): name = "test" - async def yield_seeds(self): + async def start(self): raise CloseSpider("foo") yield # pylint: disable=unreachable diff --git a/tests/test_cmdline_crawl_with_pipeline/__init__.py b/tests/test_cmdline_crawl_with_pipeline/__init__.py index 5228f6abd..5006e3689 100644 --- a/tests/test_cmdline_crawl_with_pipeline/__init__.py +++ b/tests/test_cmdline_crawl_with_pipeline/__init__.py @@ -2,17 +2,26 @@ import sys from pathlib import Path from subprocess import PIPE, Popen +from .. import TWISTED_KEEPS_TRACEBACKS + class TestCmdlineCrawlPipeline: def _execute(self, spname): args = (sys.executable, "-m", "scrapy.cmdline", "crawl", spname) cwd = Path(__file__).resolve().parent proc = Popen(args, stdout=PIPE, stderr=PIPE, cwd=cwd) - proc.communicate() - return proc.returncode + _, stderr = proc.communicate() + return proc.returncode, stderr def test_open_spider_normally_in_pipeline(self): - assert self._execute("normal") == 0 + returncode, stderr = self._execute("normal") + assert returncode == 0 def test_exception_at_open_spider_in_pipeline(self): - assert self._execute("exception") == 1 + returncode, stderr = self._execute("exception") + # An unhandled exception in a pipeline should not stop the crawl + assert returncode == 0 + if TWISTED_KEEPS_TRACEBACKS: + assert b'RuntimeError("exception")' in stderr + else: + assert b"RuntimeError: exception" in stderr diff --git a/tests/test_commands.py b/tests/test_commands.py index 0cac5b24d..16af97842 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -670,12 +670,22 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug("It Works!") return yield """ + badspider = """ +import scrapy + +class BadSpider(scrapy.Spider): + name = "bad" + async def start(self): + raise Exception("oops!") + yield + """ + @contextmanager def _create_file(self, content: str, name: str | None = None) -> Iterator[str]: with TemporaryDirectory() as tmpdir: @@ -763,6 +773,11 @@ class MySpider(scrapy.Spider): log = self.get_log("", name="myspider.txt") assert "Unable to load" in log + def test_start_errors(self): + log = self.get_log(self.badspider, name="badspider.py") + assert "start" in log + assert "badspider.py" in log, log + def test_asyncio_enabled_true(self): log = self.get_log( self.debug_log_spider, @@ -833,7 +848,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug('FEEDS: {}'.format(self.settings.getdict('FEEDS'))) return yield @@ -850,7 +865,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug( 'FEEDS: {}'.format( json.dumps(self.settings.getdict('FEEDS'), sort_keys=True) @@ -877,7 +892,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): return yield """ @@ -894,7 +909,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug('FEEDS: {}'.format(self.settings.getdict('FEEDS'))) return yield @@ -974,7 +989,7 @@ class MySpider(scrapy.Spider): spider.settings.set("FOO", kwargs.get("foo")) return spider - async def yield_seeds(self): + async def start(self): self.logger.info(f"The value of FOO is {self.settings.getint('FOO')}") return yield @@ -993,6 +1008,11 @@ class TestWindowsRunSpiderCommand(TestRunSpiderCommand): raise unittest.SkipTest("Windows required for .pyw files") return super().setUp() + def test_start_errors(self): + log = self.get_log(self.badspider, name="badspider.pyw") + assert "start" in log + assert "badspider.pyw" in log + def test_runspider_unable_to_load(self): raise unittest.SkipTest("Already Tested in 'RunSpiderCommandTest' ") @@ -1040,7 +1060,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug('It works!') return yield @@ -1055,7 +1075,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug('FEEDS: {}'.format(self.settings.getdict('FEEDS'))) return yield @@ -1072,7 +1092,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): self.logger.debug( 'FEEDS: {}'.format( json.dumps(self.settings.getdict('FEEDS'), sort_keys=True) @@ -1099,7 +1119,7 @@ import scrapy class MySpider(scrapy.Spider): name = 'myspider' - async def yield_seeds(self): + async def start(self): return yield """ diff --git a/tests/test_contracts.py b/tests/test_contracts.py index d06e186cf..26b16a1d4 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -511,9 +511,9 @@ class TestContractsManager(unittest.TestCase): super().__init__(*args, **kwargs) self.visited = 0 - async def yield_seeds(self_): # pylint: disable=no-self-argument - for seed in self.conman.from_spider(self_, self.results): - yield seed + async def start(self_): # pylint: disable=no-self-argument + for item_or_request in self.conman.from_spider(self_, self.results): + yield item_or_request def parse_first(self, response): self.visited += 1 diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 31c5618ad..7793505da 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -34,6 +34,7 @@ from tests.spiders import ( AsyncDefDeferredMaybeWrappedSpider, AsyncDefDeferredWrappedSpider, AsyncDefSpider, + BrokenStartSpider, BytesReceivedCallbackSpider, BytesReceivedErrbackSpider, CrawlSpiderWithAsyncCallback, @@ -42,14 +43,14 @@ from tests.spiders import ( CrawlSpiderWithParseMethod, CrawlSpiderWithProcessRequestCallbackKeywordArguments, DelaySpider, - DuplicateYieldSeedsSpider, + DuplicateStartSpider, FollowAllSpider, HeadersReceivedCallbackSpider, HeadersReceivedErrbackSpider, SimpleSpider, SingleRequestSpider, - YieldSeedsGoodAndBadOutput, - YieldSeedsItemSpider, + StartGoodAndBadOutput, + StartItemSpider, ) @@ -162,36 +163,57 @@ class TestCrawl(TestCase): self._assert_retried(log) @defer.inlineCallbacks - def test_yield_seeds_items(self): + def test_start_bug_before_yield(self): with LogCapture("scrapy", level=logging.ERROR) as log: - crawler = get_crawler(YieldSeedsItemSpider) + crawler = get_crawler(BrokenStartSpider) + yield crawler.crawl(fail_before_yield=1, mockserver=self.mockserver) + + assert len(log.records) == 1 + record = log.records[0] + assert record.exc_info is not None + assert record.exc_info[0] is ZeroDivisionError + + @defer.inlineCallbacks + def test_start_bug_yielding(self): + with LogCapture("scrapy", level=logging.ERROR) as log: + crawler = get_crawler(BrokenStartSpider) + yield crawler.crawl(fail_yielding=1, mockserver=self.mockserver) + + assert len(log.records) == 1 + record = log.records[0] + assert record.exc_info is not None + assert record.exc_info[0] is ZeroDivisionError + + @defer.inlineCallbacks + def test_start_items(self): + with LogCapture("scrapy", level=logging.ERROR) as log: + crawler = get_crawler(StartItemSpider) yield crawler.crawl(mockserver=self.mockserver) assert len(log.records) == 0 @defer.inlineCallbacks - def test_yield_seeds_unsupported_output(self): - """Anything that is not a request, a seeding policy or a string (which - is assumed to be a seeding policy) is assumed to be an item, avoiding a - potentially expensive call to itemadapter.is_item, and letting instead - things fail when ItemAdapter is actually used on the corresponding - non-item object.""" + def test_start_unsupported_output(self): + """Anything that is not a request is assumed to be an item, avoiding a + potentially expensive call to itemadapter.is_item(), and letting + instead things fail when ItemAdapter is actually used on the + corresponding non-item object.""" with LogCapture("scrapy", level=logging.ERROR) as log: - crawler = get_crawler(YieldSeedsGoodAndBadOutput) + crawler = get_crawler(StartGoodAndBadOutput) yield crawler.crawl(mockserver=self.mockserver) - assert len(log.records) == 1 + assert len(log.records) == 0 @defer.inlineCallbacks - def test_yield_seeds_dupes(self): + def test_start_dupes(self): settings = {"CONCURRENT_REQUESTS": 1} - crawler = get_crawler(DuplicateYieldSeedsSpider, settings) + crawler = get_crawler(DuplicateStartSpider, settings) yield crawler.crawl( dont_filter=True, distinct_urls=2, dupe_factor=3, mockserver=self.mockserver ) assert crawler.spider.visited == 6 - crawler = get_crawler(DuplicateYieldSeedsSpider, settings) + crawler = get_crawler(DuplicateStartSpider, settings) yield crawler.crawl( dont_filter=False, distinct_urls=3, @@ -273,10 +295,10 @@ with multiples lines # basic asserts in case of weird communication errors assert "responses" in crawler.spider.meta assert "failures" not in crawler.spider.meta - # test_yield_seeds doesn't set Referer header + # start() doesn't set Referer header echo0 = json.loads(to_unicode(crawler.spider.meta["responses"][2].body)) assert "Referer" not in echo0["headers"] - # following request sets Referer to test_yield_seeds url + # following request sets Referer to the source request url echo1 = json.loads(to_unicode(crawler.spider.meta["responses"][1].body)) assert echo1["headers"].get("Referer") == [req0.url] # next request avoids Referer header diff --git a/tests/test_crawler.py b/tests/test_crawler.py index fa1c90880..42950ea94 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -153,7 +153,7 @@ class TestCrawler(TestBaseCrawler): super().__init__(**kwargs) self.crawler = crawler - async def yield_seeds(self): + async def start(self): MySpider.result = crawler.get_downloader_middleware(MySpider.cls) return yield @@ -233,7 +233,7 @@ class TestCrawler(TestBaseCrawler): super().__init__(**kwargs) self.crawler = crawler - async def yield_seeds(self): + async def start(self): MySpider.result = crawler.get_extension(MySpider.cls) return yield @@ -313,7 +313,7 @@ class TestCrawler(TestBaseCrawler): super().__init__(**kwargs) self.crawler = crawler - async def yield_seeds(self): + async def start(self): MySpider.result = crawler.get_item_pipeline(MySpider.cls) return yield @@ -393,7 +393,7 @@ class TestCrawler(TestBaseCrawler): super().__init__(**kwargs) self.crawler = crawler - async def yield_seeds(self): + async def start(self): MySpider.result = crawler.get_spider_middleware(MySpider.cls) return yield @@ -580,7 +580,7 @@ class ExceptionSpider(scrapy.Spider): class NoRequestsSpider(scrapy.Spider): name = "no_request" - async def yield_seeds(self): + async def start(self): return yield diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index 5d1161750..6c061b330 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -12,6 +12,11 @@ from scrapy.core.downloader.middleware import DownloaderMiddlewareManager from scrapy.exceptions import _InvalidOutput from scrapy.http import Request, Response from scrapy.spiders import Spider +from scrapy.utils.defer import ( + deferred_f_from_coro_f, + deferred_to_future, + maybe_deferred_to_future, +) from scrapy.utils.python import to_bytes from scrapy.utils.test import get_crawler, get_from_asyncio_queue @@ -29,7 +34,7 @@ class TestManagerBase(TestCase): def tearDown(self): return self.crawler.engine.close_spider(self.spider) - def _download(self, request, response=None): + async def _download(self, request, response=None): """Executes downloader mw manager's download method and returns the result (Request or Response) or raise exception in case of failure. @@ -44,7 +49,7 @@ class TestManagerBase(TestCase): # catch deferred result and return the value results = [] dfd.addBoth(results.append) - self._wait(dfd) + await maybe_deferred_to_future(dfd) ret = results[0] if isinstance(ret, Failure): ret.raiseException() @@ -54,13 +59,15 @@ class TestManagerBase(TestCase): class TestDefaults(TestManagerBase): """Tests default behavior with default settings""" - def test_request_response(self): + @deferred_f_from_coro_f + async def test_request_response(self): req = Request("http://example.com/index.html") resp = Response(req.url, status=200) - ret = self._download(req, resp) + ret = await self._download(req, resp) assert isinstance(ret, Response), "Non-response returned" - def test_3xx_and_invalid_gzipped_body_must_redirect(self): + @deferred_f_from_coro_f + async def test_3xx_and_invalid_gzipped_body_must_redirect(self): """Regression test for a failure when redirecting a compressed request. @@ -85,13 +92,14 @@ class TestDefaults(TestManagerBase): "Location": "http://example.com/login", }, ) - ret = self._download(request=req, response=resp) + ret = await self._download(request=req, response=resp) assert isinstance(ret, Request), f"Not redirected: {ret!r}" assert to_bytes(ret.url) == resp.headers["Location"], ( "Not redirected to location header" ) - def test_200_and_invalid_gzipped_body_must_fail(self): + @deferred_f_from_coro_f + async def test_200_and_invalid_gzipped_body_must_fail(self): req = Request("http://example.com") body = b"<p>You are being redirected</p>" resp = Response( @@ -106,13 +114,14 @@ class TestDefaults(TestManagerBase): }, ) with pytest.raises(BadGzipFile): - self._download(request=req, response=resp) + await self._download(request=req, response=resp) class TestResponseFromProcessRequest(TestManagerBase): """Tests middleware returning a response from process_request.""" - def test_download_func_not_called(self): + @deferred_f_from_coro_f + async def test_download_func_not_called(self): resp = Response("http://example.com/index.html") class ResponseMiddleware: @@ -126,7 +135,7 @@ class TestResponseFromProcessRequest(TestManagerBase): dfd = self.mwman.download(download_func, req, self.spider) results = [] dfd.addBoth(results.append) - self._wait(dfd) + await maybe_deferred_to_future(dfd) assert results[0] is resp assert not download_func.called @@ -195,7 +204,8 @@ class TestProcessExceptionInvalidOutput(TestManagerBase): class TestMiddlewareUsingDeferreds(TestManagerBase): """Middlewares using Deferreds should work""" - def test_deferred(self): + @deferred_f_from_coro_f + async def test_deferred(self): resp = Response("http://example.com/index.html") class DeferredMiddleware: @@ -214,7 +224,7 @@ class TestMiddlewareUsingDeferreds(TestManagerBase): dfd = self.mwman.download(download_func, req, self.spider) results = [] dfd.addBoth(results.append) - self._wait(dfd) + await maybe_deferred_to_future(dfd) assert results[0] is resp assert not download_func.called @@ -224,7 +234,8 @@ class TestMiddlewareUsingDeferreds(TestManagerBase): class TestMiddlewareUsingCoro(TestManagerBase): """Middlewares using asyncio coroutines should work""" - def test_asyncdef(self): + @deferred_f_from_coro_f + async def test_asyncdef(self): resp = Response("http://example.com/index.html") class CoroMiddleware: @@ -238,13 +249,14 @@ class TestMiddlewareUsingCoro(TestManagerBase): dfd = self.mwman.download(download_func, req, self.spider) results = [] dfd.addBoth(results.append) - self._wait(dfd) + await maybe_deferred_to_future(dfd) assert results[0] is resp assert not download_func.called @pytest.mark.only_asyncio - def test_asyncdef_asyncio(self): + @deferred_f_from_coro_f + async def test_asyncdef_asyncio(self): resp = Response("http://example.com/index.html") class CoroMiddleware: @@ -258,7 +270,7 @@ class TestMiddlewareUsingCoro(TestManagerBase): dfd = self.mwman.download(download_func, req, self.spider) results = [] dfd.addBoth(results.append) - self._wait(dfd) + await deferred_to_future(dfd) assert results[0] is resp assert not download_func.called diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py index ad335f852..9518f1835 100644 --- a/tests/test_downloadermiddleware_robotstxt.py +++ b/tests/test_downloadermiddleware_robotstxt.py @@ -1,9 +1,11 @@ -from typing import Any +from __future__ import annotations + +from typing import TYPE_CHECKING from unittest import mock import pytest from twisted.internet import error, reactor -from twisted.internet.defer import Deferred, DeferredList, maybeDeferred +from twisted.internet.defer import Deferred, maybeDeferred from twisted.python import failure from twisted.trial import unittest @@ -13,8 +15,12 @@ from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response, TextResponse from scrapy.http.request import NO_CALLBACK from scrapy.settings import Settings +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from tests.test_robotstxt_interface import rerp_available +if TYPE_CHECKING: + from scrapy.crawler import Crawler + class TestRobotsTxtMiddleware(unittest.TestCase): def setUp(self): @@ -31,7 +37,7 @@ class TestRobotsTxtMiddleware(unittest.TestCase): with pytest.raises(NotConfigured): RobotsTxtMiddleware(self.crawler) - def _get_successful_crawler(self): + def _get_successful_crawler(self) -> Crawler: crawler = self.crawler crawler.settings.set("ROBOTSTXT_OBEY", True) ROBOTS = """ @@ -54,54 +60,41 @@ Disallow: /some/randome/page.html crawler.engine.download.side_effect = return_response return crawler - def test_robotstxt(self): + @deferred_f_from_coro_f + async def test_robotstxt(self): middleware = RobotsTxtMiddleware(self._get_successful_crawler()) - return DeferredList( - [ - self.assertNotIgnored(Request("http://site.local/allowed"), middleware), - maybeDeferred(self.assertRobotsTxtRequested, "http://site.local"), - self.assertIgnored(Request("http://site.local/admin/main"), middleware), - self.assertIgnored(Request("http://site.local/static/"), middleware), - self.assertIgnored( - Request("http://site.local/wiki/K%C3%A4ytt%C3%A4j%C3%A4:"), - middleware, - ), - self.assertIgnored( - Request("http://site.local/wiki/Käyttäjä:"), middleware - ), - ], - fireOnOneErrback=True, + await self.assertNotIgnored(Request("http://site.local/allowed"), middleware) + self.assertRobotsTxtRequested("http://site.local") + await self.assertIgnored(Request("http://site.local/admin/main"), middleware) + await self.assertIgnored(Request("http://site.local/static/"), middleware) + await self.assertIgnored( + Request("http://site.local/wiki/K%C3%A4ytt%C3%A4j%C3%A4:"), middleware + ) + await self.assertIgnored( + Request("http://site.local/wiki/Käyttäjä:"), middleware ) - def test_robotstxt_ready_parser(self): + @deferred_f_from_coro_f + async def test_robotstxt_ready_parser(self): middleware = RobotsTxtMiddleware(self._get_successful_crawler()) - d = self.assertNotIgnored(Request("http://site.local/allowed"), middleware) - d.addCallback( - lambda _: self.assertNotIgnored( - Request("http://site.local/allowed"), middleware - ) - ) - return d + await self.assertNotIgnored(Request("http://site.local/allowed"), middleware) + await self.assertNotIgnored(Request("http://site.local/allowed"), middleware) - def test_robotstxt_meta(self): + @deferred_f_from_coro_f + async def test_robotstxt_meta(self): middleware = RobotsTxtMiddleware(self._get_successful_crawler()) meta = {"dont_obey_robotstxt": True} - return DeferredList( - [ - self.assertNotIgnored( - Request("http://site.local/allowed", meta=meta), middleware - ), - self.assertNotIgnored( - Request("http://site.local/admin/main", meta=meta), middleware - ), - self.assertNotIgnored( - Request("http://site.local/static/", meta=meta), middleware - ), - ], - fireOnOneErrback=True, + await self.assertNotIgnored( + Request("http://site.local/allowed", meta=meta), middleware + ) + await self.assertNotIgnored( + Request("http://site.local/admin/main", meta=meta), middleware + ) + await self.assertNotIgnored( + Request("http://site.local/static/", meta=meta), middleware ) - def _get_garbage_crawler(self): + def _get_garbage_crawler(self) -> Crawler: crawler = self.crawler crawler.settings.set("ROBOTSTXT_OBEY", True) response = Response( @@ -116,22 +109,16 @@ Disallow: /some/randome/page.html crawler.engine.download.side_effect = return_response return crawler - def test_robotstxt_garbage(self): + @deferred_f_from_coro_f + async def test_robotstxt_garbage(self): # garbage response should be discarded, equal 'allow all' middleware = RobotsTxtMiddleware(self._get_garbage_crawler()) - return DeferredList( - [ - self.assertNotIgnored(Request("http://site.local"), middleware), - self.assertNotIgnored(Request("http://site.local/allowed"), middleware), - self.assertNotIgnored( - Request("http://site.local/admin/main"), middleware - ), - self.assertNotIgnored(Request("http://site.local/static/"), middleware), - ], - fireOnOneErrback=True, - ) + await self.assertNotIgnored(Request("http://site.local"), middleware) + await self.assertNotIgnored(Request("http://site.local/allowed"), middleware) + await self.assertNotIgnored(Request("http://site.local/admin/main"), middleware) + await self.assertNotIgnored(Request("http://site.local/static/"), middleware) - def _get_emptybody_crawler(self): + def _get_emptybody_crawler(self) -> Crawler: crawler = self.crawler crawler.settings.set("ROBOTSTXT_OBEY", True) response = Response("http://site.local/robots.txt") @@ -144,21 +131,16 @@ Disallow: /some/randome/page.html crawler.engine.download.side_effect = return_response return crawler - def test_robotstxt_empty_response(self): + @deferred_f_from_coro_f + async def test_robotstxt_empty_response(self): # empty response should equal 'allow all' middleware = RobotsTxtMiddleware(self._get_emptybody_crawler()) - return DeferredList( - [ - self.assertNotIgnored(Request("http://site.local/allowed"), middleware), - self.assertNotIgnored( - Request("http://site.local/admin/main"), middleware - ), - self.assertNotIgnored(Request("http://site.local/static/"), middleware), - ], - fireOnOneErrback=True, - ) + await self.assertNotIgnored(Request("http://site.local/allowed"), middleware) + await self.assertNotIgnored(Request("http://site.local/admin/main"), middleware) + await self.assertNotIgnored(Request("http://site.local/static/"), middleware) - def test_robotstxt_error(self): + @deferred_f_from_coro_f + async def test_robotstxt_error(self): self.crawler.settings.set("ROBOTSTXT_OBEY", True) err = error.DNSLookupError("Robotstxt address not found") @@ -171,15 +153,13 @@ Disallow: /some/randome/page.html middleware = RobotsTxtMiddleware(self.crawler) middleware._logerror = mock.MagicMock(side_effect=middleware._logerror) - deferred = middleware.process_request(Request("http://site.local"), None) + await maybe_deferred_to_future( + middleware.process_request(Request("http://site.local"), None) + ) + assert middleware._logerror.called - def check_called(_: Any) -> None: - assert middleware._logerror.called - - deferred.addCallback(check_called) - return deferred - - def test_robotstxt_immediate_error(self): + @deferred_f_from_coro_f + async def test_robotstxt_immediate_error(self): self.crawler.settings.set("ROBOTSTXT_OBEY", True) err = error.DNSLookupError("Robotstxt address not found") @@ -191,9 +171,10 @@ Disallow: /some/randome/page.html self.crawler.engine.download.side_effect = immediate_failure middleware = RobotsTxtMiddleware(self.crawler) - return self.assertNotIgnored(Request("http://site.local"), middleware) + await self.assertNotIgnored(Request("http://site.local"), middleware) - def test_ignore_robotstxt_request(self): + @deferred_f_from_coro_f + async def test_ignore_robotstxt_request(self): self.crawler.settings.set("ROBOTSTXT_OBEY", True) def ignore_request(request): @@ -206,13 +187,8 @@ Disallow: /some/randome/page.html middleware = RobotsTxtMiddleware(self.crawler) mw_module_logger.error = mock.MagicMock() - d = self.assertNotIgnored(Request("http://site.local/allowed"), middleware) - - def check_not_called(_: Any) -> None: - assert not mw_module_logger.error.called # type: ignore[attr-defined] - - d.addCallback(check_not_called) - return d + await self.assertNotIgnored(Request("http://site.local/allowed"), middleware) + assert not mw_module_logger.error.called # type: ignore[attr-defined] def test_robotstxt_user_agent_setting(self): crawler = self._get_successful_crawler() @@ -236,19 +212,27 @@ Disallow: /some/randome/page.html Deferred, ) - def assertNotIgnored(self, request, middleware): + async def assertNotIgnored( + self, request: Request, middleware: RobotsTxtMiddleware + ) -> None: spider = None # not actually used - dfd = maybeDeferred(middleware.process_request, request, spider) - dfd.addCallback(self.assertIsNone) - return dfd + result = await maybe_deferred_to_future( + maybeDeferred(middleware.process_request, request, spider) # type: ignore[call-overload] + ) + assert result is None - def assertIgnored(self, request, middleware): + async def assertIgnored( + self, request: Request, middleware: RobotsTxtMiddleware + ) -> None: spider = None # not actually used - return self.assertFailure( - maybeDeferred(middleware.process_request, request, spider), IgnoreRequest + await maybe_deferred_to_future( + self.assertFailure( + middleware.process_request(request, spider), # type: ignore[arg-type] + IgnoreRequest, + ) ) - def assertRobotsTxtRequested(self, base_url): + def assertRobotsTxtRequested(self, base_url: str) -> None: calls = self.crawler.engine.download.call_args_list request = calls[0][0][0] assert request.url == f"{base_url}/robots.txt" diff --git a/tests/test_downloaderslotssettings.py b/tests/test_downloaderslotssettings.py index d485087ae..78c83ea83 100644 --- a/tests/test_downloaderslotssettings.py +++ b/tests/test_downloaderslotssettings.py @@ -28,7 +28,7 @@ class DownloaderSlotsSettingsTestSpider(MetaSpider): }, } - async def yield_seeds(self): + async def start(self): self.times = {None: []} slots = [*self.custom_settings.get("DOWNLOAD_SLOTS", {}), None] diff --git a/tests/test_engine.py b/tests/test_engine.py index 155060bf0..de6fb1175 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -92,7 +92,7 @@ class MySpider(Spider): class DupeFilterSpider(MySpider): - async def yield_seeds(self): + async def start(self): for url in self.start_urls: yield Request(url) # no dont_filter=True @@ -150,7 +150,6 @@ class CrawlerRun: """A class to run the crawler and keep track of events occurred""" def __init__(self, spider_class): - self.spider = None self.respplug = [] self.reqplug = [] self.reqdropped = [] @@ -191,7 +190,6 @@ class CrawlerRun: self.response_downloaded, signals.response_downloaded ) self.crawler.crawl(start_urls=start_urls) - self.spider = self.crawler.spider self.deferred = defer.Deferred() dispatcher.connect(self.stop, signals.engine_stopped) @@ -297,7 +295,7 @@ class TestEngineBase(unittest.TestCase): assert len(run.itemerror) == 2 for item, response, spider, failure in run.itemerror: assert failure.value.__class__ is ZeroDivisionError - assert spider == run.spider + assert spider == run.crawler.spider assert item["url"] == response.url if "item1.html" in item["url"]: @@ -378,11 +376,14 @@ class TestEngineBase(unittest.TestCase): assert signals.spider_closed in run.signals_caught assert signals.headers_received in run.signals_caught - assert {"spider": run.spider} == run.signals_caught[signals.spider_opened] - assert {"spider": run.spider} == run.signals_caught[signals.spider_idle] - assert {"spider": run.spider, "reason": "finished"} == run.signals_caught[ - signals.spider_closed + assert {"spider": run.crawler.spider} == run.signals_caught[ + signals.spider_opened ] + assert {"spider": run.crawler.spider} == run.signals_caught[signals.spider_idle] + assert { + "spider": run.crawler.spider, + "reason": "finished", + } == run.signals_caught[signals.spider_closed] class TestEngine(TestEngineBase): @@ -420,9 +421,10 @@ class TestEngine(TestEngineBase): def test_crawler_change_close_reason_on_idle(self): run = CrawlerRun(ChangeCloseReasonSpider) yield run.run() - assert {"spider": run.spider, "reason": "custom_reason"} == run.signals_caught[ - signals.spider_closed - ] + assert { + "spider": run.crawler.spider, + "reason": "custom_reason", + } == run.signals_caught[signals.spider_closed] @defer.inlineCallbacks def test_close_downloader(self): @@ -472,7 +474,7 @@ class TestEngine(TestEngineBase): finally: timer.cancel() - assert b"Traceback" not in stderr + assert b"Traceback" not in stderr, stderr def test_request_scheduled_signal(caplog): @@ -486,11 +488,11 @@ def test_request_scheduled_signal(caplog): engine.downloader._slot_gc_loop.stop() scheduler = MemoryScheduler() - async def seeds(): + async def start(): return yield - engine._seeds = seeds() + engine._start = start() engine._slot = _Slot(False, Mock(), scheduler) crawler.signals.connect(signal_handler, request_scheduled) keep_request = Request("https://keep.example") diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 7ff5bcaff..e4ea3cbc2 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -3,11 +3,12 @@ from __future__ import annotations from collections import deque from logging import ERROR +import pytest from testfixtures import LogCapture from twisted.internet.defer import Deferred from twisted.trial.unittest import TestCase -from scrapy import Request, SeedingPolicy, Spider, signals +from scrapy import Request, Spider, signals from scrapy.core.engine import ExecutionEngine from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler @@ -16,127 +17,24 @@ from .mockserver import MockServer from .test_scheduler import MemoryScheduler, PriorityScheduler -def sleep(seconds: float = ExecutionEngine._MIN_BACK_IN_SECONDS): +async def sleep(seconds: float = ExecutionEngine._MIN_BACK_IN_SECONDS) -> None: from twisted.internet import reactor deferred: Deferred[None] = Deferred() reactor.callLater(seconds, deferred.callback, None) - return maybe_deferred_to_future(deferred) + await maybe_deferred_to_future(deferred) class MainTestCase(TestCase): @deferred_f_from_coro_f - async def test_greedy(self): - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield Request("data:,a") - self.crawler.engine._slot.scheduler.enqueue_request(Request("data:,b")) - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": MemoryScheduler} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b"] - assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" - - @deferred_f_from_coro_f - async def test_greedy_sleep(self): - """If the seeds sleep long enough, scheduler requests should be - processed in the meantime.""" - - # This value may need an increase depending on how slow CI jobs can be. - # Just increase the last integer by one until the test passes. - seconds = ExecutionEngine._MIN_BACK_IN_SECONDS * 2**3 - - class TestScheduler(MemoryScheduler): - queue = ["data:,b"] + async def test_start_exception(self): + """If Spider.start() raises an unhandled exception, scheduler requests + should still be processed.""" class TestSpider(Spider): name = "test" - async def yield_seeds(self): - yield Request("data:,a") - await sleep(seconds) - yield Request("data:,c") - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": TestScheduler} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) - with LogCapture() as log: - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b", "data:,c"] - assert actual_urls == expected_urls, ( - f"{actual_urls=} != {expected_urls=}\n{log}" - ) - - @deferred_f_from_coro_f - async def test_greedy_scheduler_sleep(self): - """If the scheduler sleeps but not longer than the seeds, its - processing should resume before that of the seeds, instead of being - blocked by the seeds finishing processing.""" - - class TestScheduler(MemoryScheduler): - pause = True - queue = ["data:,a"] - - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - seconds = ExecutionEngine._MIN_BACK_IN_SECONDS - await sleep(seconds) - self.crawler.engine._slot.scheduler.pause = False - await sleep(seconds) - yield Request("data:,b") - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": TestScheduler} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) - with LogCapture() as log: - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b"] - assert actual_urls == expected_urls, ( - f"{actual_urls=} != {expected_urls=}\n{log}" - ) - - @deferred_f_from_coro_f - async def test_greedy_exception(self): - """If the seeds raise an unhandled exception, scheduler requests should - still be processed.""" - - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): + async def start(self): yield Request("data:,a") self.crawler.engine._slot.scheduler.enqueue_request(Request("data:,b")) raise RuntimeError @@ -161,45 +59,14 @@ class MainTestCase(TestCase): ) @deferred_f_from_coro_f - async def test_lazy(self): - class TestScheduler(MemoryScheduler): - queue = ["data:,a"] - - class TestSpider(Spider): - name = "test" - start_urls = ["data:,b"] - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "lazy"} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b"] - assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" - - @deferred_f_from_coro_f - async def test_lazy_sleep(self): - """If the scheduler reports having requests but yields none, the lazy - policy schedules requests from seeds.""" - - class TestScheduler(MemoryScheduler): - queue = ["data:,b"] - pause = True - + async def test_close_during_start_iteration(self): class TestSpider(Spider): name = "test" - async def yield_seeds(self): + async def start(self): + assert self.crawler.engine is not None + await maybe_deferred_to_future(self.crawler.engine.close()) yield Request("data:,a") - self.crawler.engine._slot.scheduler.pause = False def parse(self, response): pass @@ -209,105 +76,17 @@ class MainTestCase(TestCase): def track_url(request, spider): actual_urls.append(request.url) - settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "lazy"} + settings = {"SCHEDULER": MemoryScheduler} crawler = get_crawler(TestSpider, settings_dict=settings) crawler.signals.connect(track_url, signals.request_reached_downloader) - with LogCapture() as log: - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b"] - assert actual_urls == expected_urls, ( - f"{actual_urls=} != {expected_urls=}\n{log}" - ) - @deferred_f_from_coro_f - async def test_lazy_seed_order(self): - """By default, seed requests should be sent in the order in which they - are iterated.""" - - class TestSpider(Spider): - name = "test" - start_urls = ["data:,a", "data:,b", "data:,c"] - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SEEDING_POLICY": "lazy"} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b", "data:,c"] - assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" - - @deferred_f_from_coro_f - async def test_front_load(self): - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield Request("data:,b", priority=0) - yield Request("data:,a", priority=1) - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": PriorityScheduler, "SEEDING_POLICY": "front_load"} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = ["data:,a", "data:,b"] - assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" - - @deferred_f_from_coro_f - async def test_override(self): - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield "front-load" # typo - yield SeedingPolicy.front_load - yield Request("data:,b", priority=1) - yield Request("data:,a", priority=2) - yield self.crawler.settings["SEEDING_POLICY"] - yield Request("data:,c", priority=3) - - def parse(self, response): - pass - - actual_items = [] - actual_urls = [] - - def track_item(item, response, spider): - actual_items.append(item) - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": PriorityScheduler, "SEEDING_POLICY": "lazy"} - crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_item, signals.item_scraped) - crawler.signals.connect(track_url, signals.request_reached_downloader) with LogCapture(level=ERROR) as log: await maybe_deferred_to_future(crawler.crawl()) - assert len(log.records) == 1 - assert "must be valid seeding policies" in str(log.records[0]) - assert crawler.stats.get_value("finish_reason") == "finished" - assert not actual_items, ( - f"{actual_items=} should be empty, policies are not items" - ) - expected_urls = ["data:,a", "data:,b", "data:,c"] + + assert not log.records, f"{log.records=}" + finish_reason = crawler.stats.get_value("finish_reason") + assert finish_reason == "shutdown", f"{finish_reason=}" + expected_urls = [] assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" # Unexpected scheduler exceptions @@ -391,8 +170,8 @@ class MainTestCase(TestCase): class TestSpider(Spider): name = "test" - async def yield_seeds(self): - await sleep() + async def start(self): + await sleep(ExecutionEngine._MIN_BACK_IN_SECONDS * 2) yield Request("data:,c") def parse(self, response): @@ -414,10 +193,8 @@ class MainTestCase(TestCase): assert "in next_request\n raise request" in str(log), log -class MockServerTestCase(TestCase): - # If requests are too fast, test_idle will fail because the outcome will - # match that of the lazy seeding policy. - delay = 0.2 +class RequestSendOrderTestCase(TestCase): + seconds = 0.1 # increase if flaky @classmethod def setUpClass(cls): @@ -426,8 +203,166 @@ class MockServerTestCase(TestCase): @classmethod def tearDownClass(cls): - cls.mockserver.__exit__(None, None, None) + cls.mockserver.__exit__(None, None, None) # increase if flaky + def _request(self, num, response_seconds, download_slots=1): + url = self.mockserver.url(f"/delay?n={response_seconds}&{num}") + meta = {"download_slot": str(num % download_slots)} + return Request(url, meta=meta) + + @deferred_f_from_coro_f + async def _test_request_order( + self, + start_nums, + cb_nums=None, + settings=None, + response_seconds=None, + download_slots=1, + start_fn=None, + ): + cb_nums = cb_nums or [] + settings = settings or {} + response_seconds = response_seconds or self.seconds + + if start_fn is None: + + async def start_fn(spider): + for num in start_nums: + yield self._request(num, response_seconds, download_slots) + + class TestSpider(Spider): + name = "test" + cb_requests = deque( + [ + self._request(num, response_seconds, download_slots) + for num in cb_nums + ] + ) + start = start_fn + + def parse(self, response): + while self.cb_requests: + yield self.cb_requests.popleft() + + actual_nums = [] + + def track_num(request, spider): + actual_nums.append(int(request.url.rsplit("&", maxsplit=1)[1])) + + crawler = get_crawler(TestSpider, settings_dict=settings) + crawler.signals.connect(track_num, signals.request_reached_downloader) + await maybe_deferred_to_future(crawler.crawl()) + assert crawler.stats.get_value("finish_reason") == "finished" + expected_nums = sorted(start_nums + cb_nums) + assert actual_nums == expected_nums, f"{actual_nums=} != {expected_nums=}" + + # Examples from the “Start requests” section of the documentation about + # spiders. + + @deferred_f_from_coro_f + async def test_start_requests_first(self): + start_nums = [1, 3, 2] + cb_nums = [4] + response_seconds = self.seconds + download_slots = 1 + + async def start(spider): + for num in start_nums: + request = self._request(num, response_seconds, download_slots) + yield request.replace(priority=1) + + await maybe_deferred_to_future( + self._test_request_order( + start_nums=start_nums, + cb_nums=cb_nums, + settings={"CONCURRENT_REQUESTS": 1}, + response_seconds=response_seconds, + start_fn=start, + ) + ) + + @deferred_f_from_coro_f + async def test_start_requests_first_sorted(self): + start_nums = [1, 2, 3] + cb_nums = [4] + response_seconds = self.seconds + download_slots = 1 + + async def start(spider): + priority = len(start_nums) + for num in start_nums: + request = self._request(num, response_seconds, download_slots) + yield request.replace(priority=priority) + priority -= 1 + + await maybe_deferred_to_future( + self._test_request_order( + start_nums=start_nums, + cb_nums=cb_nums, + settings={"CONCURRENT_REQUESTS": 1}, + response_seconds=response_seconds, + start_fn=start, + ) + ) + + @pytest.mark.skip(reason="not implemented yet") + @deferred_f_from_coro_f + async def test_front_load(self): + class TestSpider(Spider): + name = "test" + + async def start(self): + self.crawler.engine.scheduler.pause() + yield Request("data:,b", priority=0) + yield Request("data:,a", priority=1) + self.crawler.engine.scheduler.unpause() + + def parse(self, response): + pass + + actual_urls = [] + + def track_url(request, spider): + actual_urls.append(request.url) + + settings = {"SCHEDULER": PriorityScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + crawler.signals.connect(track_url, signals.request_reached_downloader) + await maybe_deferred_to_future(crawler.crawl()) + assert crawler.stats.get_value("finish_reason") == "finished" + expected_urls = ["data:,a", "data:,b"] + assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" + + @deferred_f_from_coro_f + async def test_lazy(self): + start_nums = [1, 2, 4] + cb_nums = [3] + response_seconds = self.seconds + download_slots = 1 + + async def start(spider): + for num in start_nums: + if spider.crawler.engine.needs_backout(): + await spider.crawler.signals.wait_for(signals.scheduler_empty) + request = self._request(num, response_seconds, download_slots) + yield request + + await maybe_deferred_to_future( + self._test_request_order( + start_nums=start_nums, + cb_nums=cb_nums, + settings={ + "CONCURRENT_REQUESTS": 1, + # Without the lazy approach, a FIFO queue would yield the + # start requests in a different order. + "SCHEDULER_MEMORY_QUEUE": "scrapy.squeues.FifoMemoryQueue", + }, + response_seconds=response_seconds, + start_fn=start, + ) + ) + + @pytest.mark.skip(reason="not implemented yet") @deferred_f_from_coro_f async def test_idle(self): def _url(id): @@ -461,3 +396,59 @@ class MockServerTestCase(TestCase): assert crawler.stats.get_value("finish_reason") == "finished" expected_urls = [_url(letter) for letter in "abcd"] assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" + + # Sleep handling + + @deferred_f_from_coro_f + async def test_sleep(self): + """Neither asynchronous sleeps on Spider.start() nor the equivalent on + the scheduler (returning no requests while also returning True from + the has_pending_requests() method) should cause the spider to miss the + processing of any later requests.""" + seconds = ExecutionEngine._MIN_BACK_IN_SECONDS + + def _request(num): + return self._request(num, seconds) + + async def _sleep(): + await sleep(seconds) + + async def start(spider): + from twisted.internet import reactor + + yield _request(1) + + # Let request 1 be processed. + await _sleep() + + spider.crawler.engine._slot.scheduler.pause() + spider.crawler.engine._slot.scheduler.enqueue_request(_request(2)) + + # During this time, the scheduler reports having requests but + # returns None. + await _sleep() + + spider.crawler.engine._slot.scheduler.unpause() + + # The scheduler request is processed. + await _sleep() + + yield _request(3) + + spider.crawler.engine._slot.scheduler.pause() + spider.crawler.engine._slot.scheduler.enqueue_request(_request(4)) + + # The last start request is processed during the time until the + # delayed call below, proving that the start iteration can + # finish before a scheduler “sleep” without causing the + # scheduler to finish. + reactor.callLater(seconds, spider.crawler.engine._slot.scheduler.unpause) + + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[1, 2, 3, 4], + settings={"SCHEDULER": MemoryScheduler}, + response_seconds=seconds, + start_fn=start, + ) + ) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index 19f260c0b..d658d1526 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -69,7 +69,7 @@ class AsyncDefNotAsyncioPipeline: class ItemSpider(Spider): name = "itemspider" - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status?n=200")) def parse(self, response): diff --git a/tests/test_request_cb_kwargs.py b/tests/test_request_cb_kwargs.py index c2c1b6288..79b53b33b 100644 --- a/tests/test_request_cb_kwargs.py +++ b/tests/test_request_cb_kwargs.py @@ -28,10 +28,10 @@ class InjectArgumentsSpiderMiddleware: Make sure spider middlewares are able to update the keyword arguments """ - def process_test_yield_seeds(self, test_yield_seeds, spider): - for request in test_yield_seeds: + async def process_start(self, start): + async for request in start: if request.callback.__name__ == "parse_spider_mw": - request.cb_kwargs["from_process_test_yield_seeds"] = True + request.cb_kwargs["from_process_start"] = True yield request def process_spider_input(self, response, spider): @@ -62,7 +62,7 @@ class KeywordArgumentsSpider(MockServerSpider): checks: list[bool] = [] - async def yield_seeds(self): + async def start(self): data = {"key": "value", "number": 123, "callback": "some_callback"} yield Request(self.mockserver.url("/first"), self.parse_first, cb_kwargs=data) yield Request( @@ -138,11 +138,9 @@ class KeywordArgumentsSpider(MockServerSpider): self.checks.append(bool(from_process_response)) self.crawler.stats.inc_value("boolean_checks", 2) - def parse_spider_mw( - self, response, from_process_spider_input, from_process_test_yield_seeds - ): + def parse_spider_mw(self, response, from_process_spider_input, from_process_start): self.checks.append(bool(from_process_spider_input)) - self.checks.append(bool(from_process_test_yield_seeds)) + self.checks.append(bool(from_process_start)) self.crawler.stats.inc_value("boolean_checks", 2) return Request(self.mockserver.url("/spider_mw_2"), self.parse_spider_mw_2) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index afd4ecc80..9775c56d2 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -22,7 +22,7 @@ from tests.mockserver import MockServer class MemoryScheduler(BaseScheduler): - pause = False + paused = False def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -36,16 +36,22 @@ class MemoryScheduler(BaseScheduler): return True def has_pending_requests(self) -> bool: - return self.pause or bool(self.queue) + return self.paused or bool(self.queue) def next_request(self) -> Request | None: - if self.pause: + if self.paused: return None try: return self.queue.pop() except IndexError: return None + def pause(self) -> None: + self.paused = True + + def unpause(self) -> None: + self.paused = False + class PriorityScheduler(BaseScheduler): def __init__(self, *args, **kwargs): diff --git a/tests/test_signals.py b/tests/test_signals.py index 576843f53..663e912b7 100644 --- a/tests/test_signals.py +++ b/tests/test_signals.py @@ -1,8 +1,9 @@ import pytest from twisted.internet import defer -from twisted.trial import unittest +from twisted.trial.unittest import TestCase from scrapy import Request, Spider, signals +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler, get_from_asyncio_queue from tests.mockserver import MockServer @@ -10,7 +11,7 @@ from tests.mockserver import MockServer class ItemSpider(Spider): name = "itemspider" - async def yield_seeds(self): + async def start(self): for index in range(10): yield Request( self.mockserver.url(f"/status?n=200&id={index}"), meta={"index": index} @@ -20,7 +21,21 @@ class ItemSpider(Spider): return {"index": response.meta["index"]} -class TestAsyncSignal(unittest.TestCase): +class MainTestCase(TestCase): + @deferred_f_from_coro_f + async def test_scheduler_empty(self): + crawler = get_crawler() + calls = [] + + def track_call(): + calls.append(object()) + + crawler.signals.connect(track_call, signals.scheduler_empty) + await maybe_deferred_to_future(crawler.crawl()) + assert len(calls) >= 1 + + +class MockServerTestCase(TestCase): @classmethod def setUpClass(cls): cls.mockserver = MockServer() diff --git a/tests/test_spider.py b/tests/test_spider.py index f0a8ad4d7..db18a11dd 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -26,6 +26,7 @@ from scrapy.spiders import ( XMLFeedSpider, ) from scrapy.spiders.init import InitSpider +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler, get_reactor_settings from tests import get_testdata, tests_datadir @@ -145,6 +146,22 @@ class TestSpider(unittest.TestCase): class TestInitSpider(TestSpider): spider_class = InitSpider + @deferred_f_from_coro_f + async def test_start_urls(self): + responses = [] + + class TestSpider(self.spider_class): + name = "test" + start_urls = ["data:,"] + + async def parse(self, response): + responses.append(response) + + crawler = get_crawler(TestSpider) + await maybe_deferred_to_future(crawler.crawl()) + assert len(responses) == 1 + assert responses[0].url == "data:," + class TestXMLFeedSpider(TestSpider): spider_class = XMLFeedSpider @@ -762,6 +779,24 @@ Sitemap: /sitemap-relative-url.xml ), ) + @deferred_f_from_coro_f + async def test_sitemap_urls(self): + class TestSpider(self.spider_class): + name = "test" + sitemap_urls = ["https://toscrape.com/sitemap.xml"] + + crawler = get_crawler(TestSpider) + spider = TestSpider.from_crawler(crawler) + with warnings.catch_warnings(): + warnings.simplefilter("error") + requests = [request async for request in spider.start()] + + assert len(requests) == 1 + request = requests[0] + assert request.url == "https://toscrape.com/sitemap.xml" + assert request.dont_filter is False + assert request.callback == spider._parse_sitemap + class TestDeprecation: def test_crawl_spider(self): diff --git a/tests/test_spider_start.py b/tests/test_spider_start.py new file mode 100644 index 000000000..3a20db922 --- /dev/null +++ b/tests/test_spider_start.py @@ -0,0 +1,302 @@ +import warnings +from asyncio import sleep + +import pytest +from testfixtures import LogCapture +from twisted.internet.defer import Deferred +from twisted.trial.unittest import TestCase + +from scrapy import Spider, signals +from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.test import get_crawler + +from . import TWISTED_KEEPS_TRACEBACKS +from .test_scheduler import MemoryScheduler + +SLEEP_SECONDS = 0.1 +ITEM_A = {"id": "a"} +ITEM_B = {"id": "b"} + + +def twisted_sleep(seconds): + from twisted.internet import reactor + + d = Deferred() + reactor.callLater(seconds, d.callback, None) + return d + + +class MainTestCase(TestCase): + # Utility methods + + async def _test_spider(self, spider, expected_items=None, settings=None): + actual_items = [] + expected_items = [] if expected_items is None else expected_items + settings = settings or {} + + def track_item(item, response, spider): + actual_items.append(item) + + crawler = get_crawler(spider, settings_dict=settings) + crawler.signals.connect(track_item, signals.item_scraped) + await maybe_deferred_to_future(crawler.crawl()) + assert crawler.stats.get_value("finish_reason") == "finished" + assert actual_items == expected_items, f"{actual_items=} != {expected_items=}" + + async def _test_start(self, start_fn, expected_items=None): + class TestSpider(Spider): + name = "test" + start = start_fn + + await self._test_spider(TestSpider, expected_items) + + # Basic usage + + @deferred_f_from_coro_f + async def test_start_urls(self): + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + + async def parse(self, response): + yield ITEM_A + + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_start(self): + class TestSpider(Spider): + name = "test" + + async def start(self): + yield ITEM_A + + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_start_subclass(self): + class BaseSpider(Spider): + async def start(self): + yield ITEM_A + + class TestSpider(BaseSpider): + name = "test" + + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_deprecated(self): + class TestSpider(Spider): + name = "test" + + def start_requests(self): + yield ITEM_A + + with pytest.warns(ScrapyDeprecationWarning): + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_deprecated_subclass(self): + class BaseSpider(Spider): + def start_requests(self): + yield ITEM_A + + class TestSpider(BaseSpider): + name = "test" + + # The warning must be about the base class and not the subclass. + with pytest.warns(ScrapyDeprecationWarning, match="BaseSpider"): + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_universal(self): + class TestSpider(Spider): + name = "test" + + async def start(self): + yield ITEM_A + + def start_requests(self): + yield ITEM_B + + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_universal_subclass(self): + class BaseSpider(Spider): + async def start(self): + yield ITEM_A + + def start_requests(self): + yield ITEM_B + + class TestSpider(BaseSpider): + name = "test" + + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_spider(TestSpider, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_start_deprecated_super(self): + class TestSpider(Spider): + name = "test" + + async def start(self): + for item_or_request in super().start_requests(): + yield item_or_request + + with pytest.warns( + ScrapyDeprecationWarning, match=r"use Spider\.start\(\) instead" + ) as messages: + await self._test_spider(TestSpider, []) + assert messages[0].filename.endswith("test_spider_start.py") + + @pytest.mark.only_asyncio + @deferred_f_from_coro_f + async def test_asyncio_delayed(self): + async def start(spider): + await sleep(SLEEP_SECONDS) + yield ITEM_A + + await self._test_start(start, [ITEM_A]) + + @deferred_f_from_coro_f + async def test_twisted_delayed(self): + async def start(spider): + await maybe_deferred_to_future(twisted_sleep(SLEEP_SECONDS)) + yield ITEM_A + + await self._test_start(start, [ITEM_A]) + + # Bad definitions. + + @deferred_f_from_coro_f + async def test_start_non_gen(self): + async def start(spider): + return + + with LogCapture() as log, warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_start(start, []) + + assert ".start must be an asynchronous generator" in str(log) + + @deferred_f_from_coro_f + async def test_start_sync(self): + def start(spider): + return + yield + + with LogCapture() as log, warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_start(start, []) + + assert ".start must be an asynchronous generator" in str(log) + + @deferred_f_from_coro_f + async def test_start_sync_non_gen(self): + def start(spider): + return [] + + with LogCapture() as log, warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_start(start, []) + + assert ".start must be an asynchronous generator" in str(log) + + @deferred_f_from_coro_f + async def test_start_requests_non_gen_exception(self): + class TestSpider(Spider): + name = "test" + + def start_requests(self): + raise RuntimeError + + with ( + LogCapture() as log, + pytest.warns( + ScrapyDeprecationWarning, + match=r"defines the deprecated start_requests\(\) method", + ), + ): + await self._test_spider(TestSpider, []) + + assert "in start_requests\n raise RuntimeError" in str(log) + + @deferred_f_from_coro_f + async def test_start_url(self): + class TestSpider(Spider): + name = "test" + start_url = "https://toscrape.com" + + with LogCapture() as log: + await self._test_spider(TestSpider, []) + + assert "Error while reading start items and requests" in str(log), log + assert "found 'start_url' attribute instead, did you miss an 's'?" in str( + log + ), log + + @deferred_f_from_coro_f + async def test_exception_before_yield(self): + async def start(spider): + raise RuntimeError + yield # pylint: disable=unreachable + + with LogCapture() as log: + await self._test_start(start, []) + + if TWISTED_KEEPS_TRACEBACKS: + assert "in start\n raise RuntimeError" in str(log), log + else: + assert "in _process_next_seed\n seed =" in str(log), log + + @deferred_f_from_coro_f + async def test_exception_after_yield(self): + async def start(spider): + yield ITEM_A + raise RuntimeError + + with LogCapture() as log: + await self._test_start(start, [ITEM_A]) + + if TWISTED_KEEPS_TRACEBACKS: + assert "in start\n raise RuntimeError" in str(log), log + else: + assert "in _process_next_seed\n seed =" in str(log), log + + @deferred_f_from_coro_f + async def test_bad_definition_continuance(self): + """Even if start (or process_start) are not correctly defined, blocking + the iteration of start items and requests, requests from the scheduler + are still consumed.""" + + class TestScheduler(MemoryScheduler): + queue = ["data:,"] + + class TestSpider(Spider): + name = "test" + + async def start(self): + return + + async def parse(self, response): + yield ITEM_A + + settings = {"SCHEDULER": TestScheduler} + + with LogCapture() as log, warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_spider(TestSpider, [ITEM_A], settings=settings) + + assert ".start must be an asynchronous generator" in str(log), log diff --git a/tests/test_spider_yield_seeds.py b/tests/test_spider_yield_seeds.py deleted file mode 100644 index cb1f76121..000000000 --- a/tests/test_spider_yield_seeds.py +++ /dev/null @@ -1,206 +0,0 @@ -import pytest -from testfixtures import LogCapture -from twisted import version as TWISTED_VERSION -from twisted.python.versions import Version -from twisted.trial.unittest import TestCase - -from scrapy import Spider, signals -from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future -from scrapy.utils.test import get_crawler - -from .test_scheduler import MemoryScheduler - -ITEM_A = {"id": "a"} -ITEM_B = {"id": "b"} - -TWISTED_KEEPS_TRACEBACKS = TWISTED_VERSION >= Version("twisted", 24, 10, 0) - - -class MainTestCase(TestCase): - # Utility methods - - async def _test_spider(self, spider, expected_items=None, settings=None): - actual_items = [] - expected_items = [] if expected_items is None else expected_items - settings = settings or {} - - def track_item(item, response, spider): - actual_items.append(item) - - crawler = get_crawler(spider, settings_dict=settings) - crawler.signals.connect(track_item, signals.item_scraped) - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - assert actual_items == expected_items, f"{actual_items=} != {expected_items=}" - - async def _test_yield_seeds(self, yield_seeds_, expected_items=None): - class TestSpider(Spider): - name = "test" - yield_seeds = yield_seeds_ - - await self._test_spider(TestSpider, expected_items) - - # Basic usage - - @deferred_f_from_coro_f - async def test_start_urls(self): - class TestSpider(Spider): - name = "test" - start_urls = ["data:,"] - - async def parse(self, response): - yield ITEM_A - - await self._test_spider(TestSpider, [ITEM_A]) - - @deferred_f_from_coro_f - async def test_main(self): - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield ITEM_A - - await self._test_spider(TestSpider, [ITEM_A]) - - # Deprecation of start_requests and universal implementation support. - - @deferred_f_from_coro_f - async def test_deprecated(self): - class TestSpider(Spider): - name = "test" - - def start_requests(self): - yield ITEM_A - - with pytest.warns(ScrapyDeprecationWarning): - await self._test_spider(TestSpider, [ITEM_A]) - - @deferred_f_from_coro_f - async def test_deprecated_subclass(self): - class BaseSpider(Spider): - def start_requests(self): - yield ITEM_A - - class TestSpider(BaseSpider): - name = "test" - - # The warning must be about the base class and not the subclass. - with pytest.warns(ScrapyDeprecationWarning, match="BaseSpider"): - await self._test_spider(TestSpider, [ITEM_A]) - - @deferred_f_from_coro_f - async def test_universal(self): - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield ITEM_A - - def start_requests(self): - yield ITEM_B - - await self._test_spider(TestSpider, [ITEM_A]) - - # Bad definitions. - - @deferred_f_from_coro_f - async def test_async_function(self): - async def yield_seeds(spider): - return - - with LogCapture() as log: - await self._test_yield_seeds(yield_seeds, []) - - assert ".yield_seeds must be an async generator function" in str(log) - - @deferred_f_from_coro_f - async def test_sync_function(self): - def yield_seeds(spider): - return [] - - with LogCapture() as log: - await self._test_yield_seeds(yield_seeds, []) - - assert ".yield_seeds must be an async generator function" in str(log) - - @deferred_f_from_coro_f - async def test_sync_generator(self): - def yield_seeds(spider): - return - yield - - with LogCapture() as log: - await self._test_yield_seeds(yield_seeds, []) - - assert ".yield_seeds must be an async generator function" in str(log) - - @deferred_f_from_coro_f - async def test_bad_definition_continuance(self): - """Even if yield_seeds (or process_seeds) are not correctly defined, - blocking the iteration of seeds, requests from the scheduler are still - consumed.""" - - class TestScheduler(MemoryScheduler): - queue = ["data:,"] - - class TestSpider(Spider): - name = "test" - - async def yield_seeds(self): - return - - async def parse(self, response): - yield ITEM_A - - settings = {"SCHEDULER": TestScheduler} - - with LogCapture() as log: - await self._test_spider(TestSpider, [ITEM_A], settings=settings) - - assert ".yield_seeds must be an async generator function" in str(log), log - - # Exceptions during iteration. - - @deferred_f_from_coro_f - async def test_exception_before_yield(self): - async def yield_seeds(spider): - raise RuntimeError - yield # pylint: disable=unreachable - - with LogCapture() as log: - await self._test_yield_seeds(yield_seeds, []) - - if TWISTED_KEEPS_TRACEBACKS: - assert "in yield_seeds\n raise RuntimeError" in str(log), log - else: - assert "in _process_next_seed\n seed =" in str(log), log - - @deferred_f_from_coro_f - async def test_exception_after_yield(self): - async def yield_seeds(spider): - yield ITEM_A - raise RuntimeError - - with LogCapture() as log: - await self._test_yield_seeds(yield_seeds, [ITEM_A]) - - if TWISTED_KEEPS_TRACEBACKS: - assert "in yield_seeds\n raise RuntimeError" in str(log), log - else: - assert "in _process_next_seed\n seed =" in str(log), log - - @deferred_f_from_coro_f - async def test_start_url(self): - class TestSpider(Spider): - name = "test" - start_url = "https://toscrape.com" - - with LogCapture() as log: - await self._test_spider(TestSpider, []) - - assert "Error while reading seeds" in str(log), log - assert "found 'start_url' attribute instead, did you miss an 's'?" in str( - log - ), log diff --git a/tests/test_spidermiddleware.py b/tests/test_spidermiddleware.py index 9a699e8f6..9cd7b44ca 100644 --- a/tests/test_spidermiddleware.py +++ b/tests/test_spidermiddleware.py @@ -7,7 +7,6 @@ from unittest import mock import pytest from testfixtures import LogCapture from twisted.internet import defer -from twisted.internet.defer import inlineCallbacks from twisted.python.failure import Failure from twisted.trial.unittest import TestCase @@ -16,7 +15,11 @@ from scrapy.exceptions import _InvalidOutput from scrapy.http import Request, Response from scrapy.spiders import Spider from scrapy.utils.asyncgen import collect_asyncgen -from scrapy.utils.defer import deferred_from_coro, maybe_deferred_to_future +from scrapy.utils.defer import ( + deferred_f_from_coro_f, + deferred_from_coro, + maybe_deferred_to_future, +) from scrapy.utils.test import get_crawler @@ -112,7 +115,7 @@ class TestProcessSpiderExceptionReRaise(TestSpiderMiddleware): class TestBaseAsyncSpiderMiddleware(TestSpiderMiddleware): """Helpers for testing sync, async and mixed middlewares. - Should work for process_spider_output and, when it's supported, process_test_yield_seeds. + Should work for process_spider_output and, when it's supported, process_start. """ ITEM_TYPE: type | tuple @@ -201,7 +204,7 @@ class ProcessSpiderExceptionSimpleIterableMiddleware: yield {"foo": 3} -class ProcessSpiderExceptionAsyncIterableMiddleware: +class ProcessSpiderExceptionAsyncIteratorMiddleware: async def process_spider_exception(self, response, exception, spider): yield {"foo": 1} d = defer.Deferred() @@ -320,23 +323,23 @@ class TestProcessSpiderOutputInvalidResult(TestBaseAsyncSpiderMiddleware): ) -class ProcessYieldSeedsSimpleMiddleware: - def process_test_yield_seeds(self, test_yield_seeds, spider): - yield from test_yield_seeds +class ProcessStartSimpleMiddleware: + async def process_start(self, start): + async for item_or_request in start: + yield item_or_request -class TestProcessSeedsSimple(TestBaseAsyncSpiderMiddleware): - """process_seeds tests for simple yield_seeds""" +class TestProcessStartSimple(TestBaseAsyncSpiderMiddleware): + """process_start tests for simple start""" ITEM_TYPE = (Request, dict) - MW_SIMPLE = ProcessYieldSeedsSimpleMiddleware + MW_SIMPLE = ProcessStartSimpleMiddleware - @inlineCallbacks - def _get_processed_seeds(self, *mw_classes): + async def _get_processed_start(self, *mw_classes): class TestSpider(Spider): name = "test" - async def yield_seeds(self): + async def start(self): for i in range(2): yield Request(f"https://example.com/{i}", dont_filter=True) yield {"name": "test item"} @@ -347,17 +350,16 @@ class TestProcessSeedsSimple(TestBaseAsyncSpiderMiddleware): ) self.spider = self.crawler._create_spider() self.mwman = SpiderMiddlewareManager.from_crawler(self.crawler) - results = yield self.mwman.process_seeds(self.spider) - return results + return await maybe_deferred_to_future(self.mwman.process_start(self.spider)) - @inlineCallbacks - def test_simple(self): + @deferred_f_from_coro_f + async def test_simple(self): """Simple mw""" - seeds = yield self._get_processed_seeds(self.MW_SIMPLE) - assert isasyncgen(seeds) - seed_list = yield deferred_from_coro(collect_asyncgen(seeds)) - assert len(seed_list) == self.RESULT_COUNT - assert isinstance(seed_list[0], self.ITEM_TYPE) + start = await self._get_processed_start(self.MW_SIMPLE) + assert isasyncgen(start) + start_list = await collect_asyncgen(start) + assert len(start_list) == self.RESULT_COUNT + assert isinstance(start_list[0], self.ITEM_TYPE) class UniversalMiddlewareNoSync: @@ -515,7 +517,7 @@ class TestProcessSpiderException(TestBaseAsyncSpiderMiddleware): MW_ASYNCGEN = ProcessSpiderOutputAsyncGenMiddleware MW_UNIVERSAL = ProcessSpiderOutputUniversalMiddleware MW_EXC_SIMPLE = ProcessSpiderExceptionSimpleIterableMiddleware - MW_EXC_ASYNCGEN = ProcessSpiderExceptionAsyncIterableMiddleware + MW_EXC_ASYNCGEN = ProcessSpiderExceptionAsyncIteratorMiddleware def _scrape_func(self, *args, **kwargs): 1 / 0 diff --git a/tests/test_spidermiddleware_httperror.py b/tests/test_spidermiddleware_httperror.py index 5e3b2a014..fd2fc3581 100644 --- a/tests/test_spidermiddleware_httperror.py +++ b/tests/test_spidermiddleware_httperror.py @@ -30,7 +30,7 @@ class _HttpErrorSpider(MockServerSpider): self.skipped = set() self.parsed = set() - async def yield_seeds(self): + async def start(self): for url in self.start_urls: yield Request(url, self.parse, errback=self.on_error) diff --git a/tests/test_spidermiddleware_output_chain.py b/tests/test_spidermiddleware_output_chain.py index f157b8c4f..20efac543 100644 --- a/tests/test_spidermiddleware_output_chain.py +++ b/tests/test_spidermiddleware_output_chain.py @@ -36,7 +36,7 @@ class RecoverySpider(Spider): }, } - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status?n=200")) def parse(self, response): @@ -73,7 +73,7 @@ class ProcessSpiderInputSpiderWithoutErrback(Spider): } } - async def yield_seeds(self): + async def start(self): yield Request(url=self.mockserver.url("/status?n=200"), callback=self.parse) def parse(self, response): @@ -83,7 +83,7 @@ class ProcessSpiderInputSpiderWithoutErrback(Spider): class ProcessSpiderInputSpiderWithErrback(ProcessSpiderInputSpiderWithoutErrback): name = "ProcessSpiderInputSpiderWithErrback" - async def yield_seeds(self): + async def start(self): yield Request( self.mockserver.url("/status?n=200"), self.parse, errback=self.errback ) @@ -103,7 +103,7 @@ class GeneratorCallbackSpider(Spider): }, } - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status?n=200")) def parse(self, response): @@ -140,7 +140,7 @@ class NotGeneratorCallbackSpider(Spider): }, } - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status?n=200")) def parse(self, response): @@ -215,7 +215,7 @@ class GeneratorOutputChainSpider(Spider): }, } - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status?n=200")) def parse(self, response): @@ -287,7 +287,7 @@ class NotGeneratorOutputChainSpider(Spider): }, } - async def yield_seeds(self): + async def start(self): yield Request(self.mockserver.url("/status?n=200")) def parse(self, response): diff --git a/tests/test_spidermiddleware_process_seeds.py b/tests/test_spidermiddleware_process_seeds.py deleted file mode 100644 index 396695c6f..000000000 --- a/tests/test_spidermiddleware_process_seeds.py +++ /dev/null @@ -1,237 +0,0 @@ -import pytest -from testfixtures import LogCapture -from twisted.trial.unittest import TestCase - -from scrapy import Spider, signals -from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future -from scrapy.utils.test import get_crawler - -from .test_spider_yield_seeds import ( - TWISTED_KEEPS_TRACEBACKS, -) - -ITEM_A = {"id": "a"} -ITEM_B = {"id": "b"} -ITEM_C = {"id": "c"} -ITEM_D = {"id": "d"} - -# Spiders and spider middlewares for MainTestCase._test_wrap - - -class ModernWrapSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield ITEM_B - - -class UniversalWrapSpider(Spider): - name = "test" - - async def yield_seeds(self): - yield ITEM_B - - def start_requests(self): - yield ITEM_D - - -class DeprecatedWrapSpider(Spider): - name = "test" - - def start_requests(self): - yield ITEM_B - - -class ModernWrapSpiderMiddleware: - async def process_seeds(self, seeds): - yield ITEM_A - async for seed in seeds: - yield seed - yield ITEM_C - - -class UniversalWrapSpiderMiddleware: - async def process_seeds(self, seeds): - yield ITEM_A - async for seed in seeds: - yield seed - yield ITEM_C - - def process_start_requests(self, seeds, spider): - yield ITEM_A - yield from seeds - yield ITEM_C - - -class DeprecatedWrapSpiderMiddleware: - def process_start_requests(self, seeds, spider): - yield ITEM_A - yield from seeds - yield ITEM_C - - -class MainTestCase(TestCase): - # Helper methods - - async def _test(self, spider_middlewares, spider_cls, expected_items): - actual_items = [] - - def track_item(item, response, spider): - actual_items.append(item) - - settings = { - "SPIDER_MIDDLEWARES": {cls: n for n, cls in enumerate(spider_middlewares)}, - } - crawler = get_crawler(spider_cls, settings_dict=settings) - crawler.signals.connect(track_item, signals.item_scraped) - await maybe_deferred_to_future(crawler.crawl()) - assert crawler.stats.get_value("finish_reason") == "finished" - assert actual_items == expected_items, f"{actual_items=} != {expected_items=}" - - async def _test_process_seeds(self, _process_seeds, expected_items=None): - class TestSpiderMiddleware: - process_seeds = _process_seeds - - class TestSpider(Spider): - name = "test" - - await self._test([TestSpiderMiddleware], TestSpider, expected_items) - - # Deprecation and universal - - async def _test_wrap(self, spider_middleware, spider_cls, expected_items=None): - expected_items = ( - [ITEM_A, ITEM_B, ITEM_C] if expected_items is None else expected_items - ) - await self._test([spider_middleware], spider_cls, expected_items) - - @deferred_f_from_coro_f - async def test_modern_mw_modern_spider(self): - await self._test_wrap(ModernWrapSpiderMiddleware, ModernWrapSpider) - - @deferred_f_from_coro_f - async def test_modern_mw_universal_spider(self): - await self._test_wrap(ModernWrapSpiderMiddleware, UniversalWrapSpider) - - @deferred_f_from_coro_f - async def test_modern_mw_deprecated_spider(self): - with pytest.warns( - ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" - ): - await self._test_wrap(ModernWrapSpiderMiddleware, DeprecatedWrapSpider) - - @deferred_f_from_coro_f - async def test_universal_mw_modern_spider(self): - await self._test_wrap(UniversalWrapSpiderMiddleware, ModernWrapSpider) - - @deferred_f_from_coro_f - async def test_universal_mw_universal_spider(self): - await self._test_wrap(UniversalWrapSpiderMiddleware, UniversalWrapSpider) - - @deferred_f_from_coro_f - async def test_universal_mw_deprecated_spider(self): - with pytest.warns( - ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" - ): - await self._test_wrap(UniversalWrapSpiderMiddleware, DeprecatedWrapSpider) - - @deferred_f_from_coro_f - async def test_deprecated_mw_modern_spider(self): - with ( - LogCapture() as log, - pytest.warns( - ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" - ), - ): - await self._test_wrap( - DeprecatedWrapSpiderMiddleware, ModernWrapSpider, expected_items=[] - ) - - assert "only compatible with (deprecated) spiders" in str(log) - - @deferred_f_from_coro_f - async def test_deprecated_mw_universal_spider(self): - with pytest.warns( - ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" - ): - await self._test_wrap( - DeprecatedWrapSpiderMiddleware, - UniversalWrapSpider, - [ITEM_A, ITEM_D, ITEM_C], - ) - - @deferred_f_from_coro_f - async def test_deprecated_mw_deprecated_spider(self): - with ( - pytest.warns( - ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" - ), - pytest.warns( - ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" - ), - ): - await self._test_wrap(DeprecatedWrapSpiderMiddleware, DeprecatedWrapSpider) - - # Bad definitions. - - @deferred_f_from_coro_f - async def test_async_function(self): - async def process_seeds(mw, seeds): - return - - with LogCapture() as log: - await self._test_process_seeds(process_seeds, []) - - assert ".process_seeds must be an async generator function" in str(log), log - - @deferred_f_from_coro_f - async def test_sync_function(self): - def process_seeds(mw, spider): - return [] - - with LogCapture() as log: - await self._test_process_seeds(process_seeds, []) - - assert ".process_seeds must be an async generator function" in str(log) - - @deferred_f_from_coro_f - async def test_sync_generator(self): - def process_seeds(mw, spider): - return - yield - - with LogCapture() as log: - await self._test_process_seeds(process_seeds, []) - - assert ".process_seeds must be an async generator function" in str(log) - - # Exceptions during iteration. - - @deferred_f_from_coro_f - async def test_exception_before_yield(self): - async def process_seeds(mw, seeds): - raise RuntimeError - yield # pylint: disable=unreachable - - with LogCapture() as log: - await self._test_process_seeds(process_seeds, []) - - if TWISTED_KEEPS_TRACEBACKS: - assert "in process_seeds\n raise RuntimeError" in str(log), log - else: - assert "in _process_next_seed\n seed =" in str(log), log - - @deferred_f_from_coro_f - async def test_exception_after_yield(self): - async def process_seeds(mw, spider): - yield ITEM_A - raise RuntimeError - - with LogCapture() as log: - await self._test_process_seeds(process_seeds, [ITEM_A]) - - if TWISTED_KEEPS_TRACEBACKS: - assert "in process_seeds\n raise RuntimeError" in str(log), log - else: - assert "in _process_next_seed\n seed =" in str(log), log diff --git a/tests/test_spidermiddleware_process_start.py b/tests/test_spidermiddleware_process_start.py new file mode 100644 index 000000000..5dcb9b73e --- /dev/null +++ b/tests/test_spidermiddleware_process_start.py @@ -0,0 +1,406 @@ +import re +import warnings +from asyncio import sleep +from logging import ERROR + +import pytest +from testfixtures import LogCapture +from twisted.trial.unittest import TestCase + +from scrapy import Spider, signals +from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.test import get_crawler + +from . import TWISTED_KEEPS_TRACEBACKS +from .test_spider_start import SLEEP_SECONDS, twisted_sleep + +ITEM_A = {"id": "a"} +ITEM_B = {"id": "b"} +ITEM_C = {"id": "c"} +ITEM_D = {"id": "d"} + + +class AsyncioSleepSpiderMiddleware: + async def process_start(self, start): + await sleep(SLEEP_SECONDS) + async for item_or_request in start: + yield item_or_request + + +class NoOpSpiderMiddleware: + async def process_start(self, start): + async for item_or_request in start: + yield item_or_request + + +class TwistedSleepSpiderMiddleware: + async def process_start(self, start): + await maybe_deferred_to_future(twisted_sleep(SLEEP_SECONDS)) + async for item_or_request in start: + yield item_or_request + + +class UniversalSpiderMiddleware: + async def process_start(self, start): + async for item_or_request in start: + yield item_or_request + + def process_start_requests(self, start_requests, spider): + raise NotImplementedError + + +# Spiders and spider middlewares for MainTestCase._test_wrap + + +class ModernWrapSpider(Spider): + name = "test" + + async def start(self): + yield ITEM_B + + +class ModernWrapSpiderSubclass(ModernWrapSpider): + name = "test" + + +class UniversalWrapSpider(Spider): + name = "test" + + async def start(self): + yield ITEM_B + + def start_requests(self): + yield ITEM_D + + +class DeprecatedWrapSpider(Spider): + name = "test" + + def start_requests(self): + yield ITEM_B + + +class ModernWrapSpiderMiddleware: + async def process_start(self, start): + yield ITEM_A + async for item_or_request in start: + yield item_or_request + yield ITEM_C + + +class UniversalWrapSpiderMiddleware: + async def process_start(self, start): + yield ITEM_A + async for item_or_request in start: + yield item_or_request + yield ITEM_C + + def process_start_requests(self, start, spider): + yield ITEM_A + yield from start + yield ITEM_C + + +class DeprecatedWrapSpiderMiddleware: + def process_start_requests(self, start, spider): + yield ITEM_A + yield from start + yield ITEM_C + + +class MainTestCase(TestCase): + # Helper methods. + + async def _test(self, spider_middlewares, spider_cls, expected_items): + actual_items = [] + + def track_item(item, response, spider): + actual_items.append(item) + + settings = { + "SPIDER_MIDDLEWARES": {cls: n for n, cls in enumerate(spider_middlewares)}, + } + crawler = get_crawler(spider_cls, settings_dict=settings) + crawler.signals.connect(track_item, signals.item_scraped) + await maybe_deferred_to_future(crawler.crawl()) + assert crawler.stats.get_value("finish_reason") == "finished" + assert actual_items == expected_items, f"{actual_items=} != {expected_items=}" + + async def _test_wrap(self, spider_middleware, spider_cls, expected_items=None): + expected_items = ( + expected_items if expected_items is not None else [ITEM_A, ITEM_B, ITEM_C] + ) + await self._test([spider_middleware], spider_cls, expected_items) + + async def _test_douple_wrap(self, smw1, smw2, spider_cls, expected_items=None): + expected_items = ( + expected_items + if expected_items is not None + else [ITEM_A, ITEM_A, ITEM_B, ITEM_C, ITEM_C] + ) + await self._test([smw1, smw2], spider_cls, expected_items) + + async def _test_process_start(self, process_start_fn, expected_items=None): + class TestSpiderMiddleware: + process_start = process_start_fn + + class TestSpider(Spider): + name = "test" + + await self._test([TestSpiderMiddleware], TestSpider, expected_items) + + # Deprecation and universal. + + @deferred_f_from_coro_f + async def test_modern_mw_modern_spider(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_wrap(ModernWrapSpiderMiddleware, ModernWrapSpider) + + @deferred_f_from_coro_f + async def test_modern_mw_universal_spider(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_wrap(ModernWrapSpiderMiddleware, UniversalWrapSpider) + + @deferred_f_from_coro_f + async def test_modern_mw_deprecated_spider(self): + with pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" + ): + await self._test_wrap(ModernWrapSpiderMiddleware, DeprecatedWrapSpider) + + @deferred_f_from_coro_f + async def test_universal_mw_modern_spider(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_wrap(UniversalWrapSpiderMiddleware, ModernWrapSpider) + + @deferred_f_from_coro_f + async def test_universal_mw_universal_spider(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_wrap(UniversalWrapSpiderMiddleware, UniversalWrapSpider) + + @deferred_f_from_coro_f + async def test_universal_mw_deprecated_spider(self): + with pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" + ): + await self._test_wrap(UniversalWrapSpiderMiddleware, DeprecatedWrapSpider) + + @deferred_f_from_coro_f + async def test_deprecated_mw_modern_spider(self): + with ( + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ), + LogCapture(level=ERROR) as log, + ): + await self._test_wrap(DeprecatedWrapSpiderMiddleware, ModernWrapSpider, []) + assert "To solve this issue" in str(log), log + + @deferred_f_from_coro_f + async def test_deprecated_mw_modern_spider_subclass(self): + with ( + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ), + LogCapture(level=ERROR) as log, + ): + await self._test_wrap( + DeprecatedWrapSpiderMiddleware, ModernWrapSpiderSubclass, [] + ) + assert re.search( + r"\S+?\.ModernWrapSpider \(inherited by \S+?.ModernWrapSpiderSubclass\) .*? only compatible with \(deprecated\) spiders", + str(log), + ), log + + @deferred_f_from_coro_f + async def test_deprecated_mw_universal_spider(self): + with pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ): + await self._test_wrap( + DeprecatedWrapSpiderMiddleware, + UniversalWrapSpider, + [ITEM_A, ITEM_D, ITEM_C], + ) + + @deferred_f_from_coro_f + async def test_deprecated_mw_deprecated_spider(self): + with ( + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ), + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" + ), + ): + await self._test_wrap(DeprecatedWrapSpiderMiddleware, DeprecatedWrapSpider) + + @deferred_f_from_coro_f + async def test_modern_mw_universal_mw_modern_spider(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_douple_wrap( + ModernWrapSpiderMiddleware, + UniversalWrapSpiderMiddleware, + ModernWrapSpider, + ) + + @deferred_f_from_coro_f + async def test_modern_mw_deprecated_mw_modern_spider(self): + with pytest.raises(ValueError, match=r"trying to combine spider middlewares"): + await self._test_douple_wrap( + ModernWrapSpiderMiddleware, + DeprecatedWrapSpiderMiddleware, + ModernWrapSpider, + ) + + @deferred_f_from_coro_f + async def test_universal_mw_deprecated_mw_modern_spider(self): + with ( + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ), + LogCapture(level=ERROR) as log, + ): + await self._test_douple_wrap( + UniversalWrapSpiderMiddleware, + DeprecatedWrapSpiderMiddleware, + ModernWrapSpider, + [], + ) + assert re.search(r"only compatible with \(deprecated\) spiders", str(log)), log + + @deferred_f_from_coro_f + async def test_modern_mw_universal_mw_universal_spider(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + await self._test_douple_wrap( + ModernWrapSpiderMiddleware, + UniversalWrapSpiderMiddleware, + UniversalWrapSpider, + ) + + @deferred_f_from_coro_f + async def test_modern_mw_deprecated_mw_universal_spider(self): + with pytest.raises(ValueError, match=r"trying to combine spider middlewares"): + await self._test_douple_wrap( + ModernWrapSpiderMiddleware, + DeprecatedWrapSpiderMiddleware, + UniversalWrapSpider, + ) + + @deferred_f_from_coro_f + async def test_universal_mw_deprecated_mw_universal_spider(self): + with pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ): + await self._test_douple_wrap( + UniversalWrapSpiderMiddleware, + DeprecatedWrapSpiderMiddleware, + UniversalWrapSpider, + [ITEM_A, ITEM_A, ITEM_D, ITEM_C, ITEM_C], + ) + + @deferred_f_from_coro_f + async def test_modern_mw_universal_mw_deprecated_spider(self): + with pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" + ): + await self._test_douple_wrap( + ModernWrapSpiderMiddleware, + UniversalWrapSpiderMiddleware, + DeprecatedWrapSpider, + ) + + @deferred_f_from_coro_f + async def test_modern_mw_deprecated_mw_deprecated_spider(self): + with pytest.raises(ValueError, match=r"trying to combine spider middlewares"): + await self._test_douple_wrap( + ModernWrapSpiderMiddleware, + DeprecatedWrapSpiderMiddleware, + DeprecatedWrapSpider, + ) + + @deferred_f_from_coro_f + async def test_universal_mw_deprecated_mw_deprecated_spider(self): + with ( + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated process_start_requests\(\)" + ), + pytest.warns( + ScrapyDeprecationWarning, match=r"deprecated start_requests\(\)" + ), + ): + await self._test_douple_wrap( + UniversalWrapSpiderMiddleware, + DeprecatedWrapSpiderMiddleware, + DeprecatedWrapSpider, + ) + + # Bad definitions. + + @deferred_f_from_coro_f + async def test_async_function(self): + async def process_start(mw, seeds): + return + + with LogCapture() as log: + await self._test_process_start(process_start, []) + + assert ".process_start must be an asynchronous generator" in str(log), log + + @deferred_f_from_coro_f + async def test_sync_function(self): + def process_start(mw, spider): + return [] + + with LogCapture() as log: + await self._test_process_start(process_start, []) + + assert ".process_start must be an asynchronous generator" in str(log) + + @deferred_f_from_coro_f + async def test_sync_generator(self): + def process_start(mw, spider): + return + yield + + with LogCapture() as log: + await self._test_process_start(process_start, []) + + assert ".process_start must be an asynchronous generator" in str(log) + + # Exceptions during iteration. + + @deferred_f_from_coro_f + async def test_exception_before_yield(self): + async def process_start(mw, seeds): + raise RuntimeError + yield # pylint: disable=unreachable + + with LogCapture() as log: + await self._test_process_start(process_start, []) + + if TWISTED_KEEPS_TRACEBACKS: + assert "in process_start\n raise RuntimeError" in str(log), log + else: + assert "in _process_next_seed\n seed =" in str(log), log + + @deferred_f_from_coro_f + async def test_exception_after_yield(self): + async def process_start(mw, spider): + yield ITEM_A + raise RuntimeError + + with LogCapture() as log: + await self._test_process_start(process_start, [ITEM_A]) + + if TWISTED_KEEPS_TRACEBACKS: + assert "in process_start\n raise RuntimeError" in str(log), log + else: + assert "in _process_next_seed\n seed =" in str(log), log diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py index 81a83c3d7..ad31e5185 100644 --- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py +++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py @@ -2,6 +2,8 @@ import warnings from functools import partial from unittest import mock +import pytest + from scrapy.utils.misc import ( is_generator_with_return_value, warn_on_generator_with_return_value, @@ -40,7 +42,24 @@ def generator_that_returns_stuff(): class TestUtilsMisc: - def test_generators_return_something(self): + @pytest.fixture + def mock_spider(self): + class MockSettings: + def __init__(self, settings_dict=None): + self.settings_dict = settings_dict or { + "WARN_ON_GENERATOR_RETURN_VALUE": True + } + + def getbool(self, name, default=False): + return self.settings_dict.get(name, default) + + class MockSpider: + def __init__(self): + self.settings = MockSettings() + + return MockSpider() + + def test_generators_return_something(self, mock_spider): def f1(): yield 1 return 2 @@ -75,30 +94,30 @@ https://example.org assert is_generator_with_return_value(i1) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_something) + warn_on_generator_with_return_value(mock_spider, top_level_return_something) assert len(w) == 1 assert ( - 'The "NoneType.top_level_return_something" method is a generator' + 'The "MockSpider.top_level_return_something" method is a generator' in str(w[0].message) ) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, f1) + warn_on_generator_with_return_value(mock_spider, f1) assert len(w) == 1 - assert 'The "NoneType.f1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.f1" method is a generator' in str(w[0].message) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, g1) + warn_on_generator_with_return_value(mock_spider, g1) assert len(w) == 1 - assert 'The "NoneType.g1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.g1" method is a generator' in str(w[0].message) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, h1) + warn_on_generator_with_return_value(mock_spider, h1) assert len(w) == 1 - assert 'The "NoneType.h1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.h1" method is a generator' in str(w[0].message) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, i1) + warn_on_generator_with_return_value(mock_spider, i1) assert len(w) == 1 - assert 'The "NoneType.i1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.i1" method is a generator' in str(w[0].message) - def test_generators_return_none(self): + def test_generators_return_none(self, mock_spider): def f2(): yield 1 @@ -142,31 +161,31 @@ https://example.org assert not is_generator_with_return_value(l2) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, f2) + warn_on_generator_with_return_value(mock_spider, f2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, g2) + warn_on_generator_with_return_value(mock_spider, g2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, h2) + warn_on_generator_with_return_value(mock_spider, h2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, i2) + warn_on_generator_with_return_value(mock_spider, i2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, j2) + warn_on_generator_with_return_value(mock_spider, j2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, k2) + warn_on_generator_with_return_value(mock_spider, k2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, l2) + warn_on_generator_with_return_value(mock_spider, l2) assert len(w) == 0 - def test_generators_return_none_with_decorator(self): + def test_generators_return_none_with_decorator(self, mock_spider): def decorator(func): def inner_func(): func() @@ -223,36 +242,36 @@ https://example.org assert not is_generator_with_return_value(l3) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, f3) + warn_on_generator_with_return_value(mock_spider, f3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, g3) + warn_on_generator_with_return_value(mock_spider, g3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, h3) + warn_on_generator_with_return_value(mock_spider, h3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, i3) + warn_on_generator_with_return_value(mock_spider, i3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, j3) + warn_on_generator_with_return_value(mock_spider, j3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, k3) + warn_on_generator_with_return_value(mock_spider, k3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, l3) + warn_on_generator_with_return_value(mock_spider, l3) assert len(w) == 0 @mock.patch( "scrapy.utils.misc.is_generator_with_return_value", new=_indentation_error ) - def test_indentation_error(self): + def test_indentation_error(self, mock_spider): with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) assert len(w) == 1 assert "Unable to determine" in str(w[0].message) @@ -262,3 +281,32 @@ https://example.org partial_cb = partial(cb, arg1=42) assert not is_generator_with_return_value(partial_cb) + + def test_warn_on_generator_with_return_value_settings_disabled(self): + class MockSettings: + def __init__(self, settings_dict=None): + self.settings_dict = settings_dict or {} + + def getbool(self, name, default=False): + return self.settings_dict.get(name, default) + + class MockSpider: + def __init__(self): + self.settings = MockSettings({"WARN_ON_GENERATOR_RETURN_VALUE": False}) + + spider = MockSpider() + + def gen_with_return(): + yield 1 + return "value" + + with warnings.catch_warnings(record=True) as w: + warn_on_generator_with_return_value(spider, gen_with_return) + assert len(w) == 0 + + spider.settings.settings_dict["WARN_ON_GENERATOR_RETURN_VALUE"] = True + + with warnings.catch_warnings(record=True) as w: + warn_on_generator_with_return_value(spider, gen_with_return) + assert len(w) == 1 + assert "is a generator" in str(w[0].message) diff --git a/tox.ini b/tox.ini index c4afe41c1..1406811d9 100644 --- a/tox.ini +++ b/tox.ini @@ -39,12 +39,12 @@ passenv = #allow tox virtualenv to upgrade pip/wheel/setuptools download = true commands = - pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report= --cov-report=term-missing --cov-report=xml --durations=10 docs scrapy tests --doctest-modules} + pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report= --cov-report=term-missing --cov-report=xml --junitxml=testenv.junit.xml -o junit_family=legacy --durations=10 docs scrapy tests --doctest-modules} install_command = python -I -m pip install -ctests/upper-constraints.txt {opts} {packages} [testenv:typing] -basepython = python3 +basepython = python3.9 deps = mypy==1.14.0 typing-extensions==4.12.2 @@ -118,7 +118,7 @@ install_command = python -I -m pip install {opts} {packages} commands = ; tests for docs fail with parsel < 1.8.0 - pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= --durations=10 scrapy tests} + pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= --junitxml=pinned.junit.xml -o junit_family=legacy --durations=10 scrapy tests} [testenv:pinned] basepython = {[pinned]basepython} @@ -254,7 +254,7 @@ deps = {[testenv]deps} botocore>=1.4.87 commands = - pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests -m requires_botocore} + pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore.junit.xml -o junit_family=legacy -m requires_botocore} [testenv:botocore-pinned] basepython = {[pinned]basepython} @@ -265,4 +265,4 @@ install_command = {[pinned]install_command} setenv = {[pinned]setenv} commands = - pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests -m requires_botocore} + pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore-pinned.junit.xml -o junit_family=legacy -m requires_botocore}