From 62c5bd1e0c3ddca1efe5588c6498041409cf2cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Mar 2025 17:36:48 +0100 Subject: [PATCH 01/13] Remove partial typing from some code examples to keep them shorter --- docs/topics/spider-middleware.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 0517a4b8e..2fc088380 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -79,7 +79,7 @@ one or more of these methods: .. code-block:: python - async def process_seeds(self, seeds) -> AsyncIterator[Any]: + async def process_seeds(self, seeds): async for seed in seeds: yield seed @@ -96,7 +96,7 @@ one or more of these methods: .. code-block:: python - def process_start_requests(self, seeds, spider) -> Iterable[Request]: + def process_start_requests(self, seeds, spider): yield from seeds .. method:: process_spider_input(response, spider) From d4dc14155c250b652151bf734c1e4509fe03e861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Mar 2025 21:09:24 +0100 Subject: [PATCH 02/13] =?UTF-8?q?AsyncIterator=20=E2=86=92=20AsyncIterable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/spider-middleware.rst | 2 +- scrapy/commands/bench.py | 4 ++-- scrapy/commands/parse.py | 4 ++-- scrapy/core/engine.py | 4 ++-- scrapy/core/spidermw.py | 4 ++-- scrapy/spiders/__init__.py | 4 ++-- scrapy/spiders/init.py | 4 ++-- scrapy/spiders/sitemap.py | 4 ++-- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 2fc088380..36b440720 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -70,7 +70,7 @@ one or more of these methods: .. class:: SpiderMiddleware - .. method:: process_seeds(seeds: AsyncIterator[Any], /) -> AsyncIterator[Any] + .. method:: process_seeds(seeds: AsyncIterable[Any], /) -> AsyncIterable[Any] :async: Iterate over the output of :meth:`~scrapy.Spider.yield_seeds` or that diff --git a/scrapy/commands/bench.py b/scrapy/commands/bench.py index fccf7c2d3..5ed09c77c 100644 --- a/scrapy/commands/bench.py +++ b/scrapy/commands/bench.py @@ -13,7 +13,7 @@ from scrapy.linkextractors import LinkExtractor if TYPE_CHECKING: import argparse - from collections.abc import AsyncIterator + from collections.abc import AsyncIterable class Command(ScrapyCommand): @@ -59,7 +59,7 @@ class _BenchSpider(scrapy.Spider): baseurl = "http://localhost:8998" link_extractor = LinkExtractor() - async def yield_seeds(self) -> AsyncIterator[Any]: + async def yield_seeds(self) -> AsyncIterable[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/parse.py b/scrapy/commands/parse.py index f0d152570..810824c7e 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -22,7 +22,7 @@ from scrapy.utils.spider import spidercls_for_request if TYPE_CHECKING: import argparse - from collections.abc import AsyncGenerator, AsyncIterator, Coroutine, Iterable + from collections.abc import AsyncGenerator, AsyncIterable, Coroutine, Iterable from twisted.python.failure import Failure @@ -258,7 +258,7 @@ 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 yield_seeds(spider: Spider) -> AsyncIterable[Any]: yield self.prepare_request(spider, Request(url), opts) if self.spidercls: diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 7cdc6287f..7383fc0e0 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -26,7 +26,7 @@ from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.reactor import CallLaterOnce if TYPE_CHECKING: - from collections.abc import AsyncIterator, Callable, Generator + from collections.abc import AsyncIterable, Callable, Generator from scrapy.core.downloader import Downloader from scrapy.core.scheduler import BaseScheduler @@ -112,7 +112,7 @@ class ExecutionEngine: ) self.start_time: float | None = None self._load_seeding_policy() - self._seeds: AsyncIterator[Any] | None = None + self._seeds: AsyncIterable[Any] | None = None def _load_seeding_policy(self) -> None: try: diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 280c38ccd..0affa4534 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 AsyncIterable, Callable, Iterable from inspect import isasyncgenfunction, iscoroutine, iscoroutinefunction from itertools import islice from typing import TYPE_CHECKING, Any, TypeVar, Union, cast @@ -371,7 +371,7 @@ class SpiderMiddlewareManager(MiddlewareManager): @inlineCallbacks def process_seeds( self, spider: Spider - ) -> Generator[Deferred[Any], Any, AsyncIterator[Any]]: + ) -> Generator[Deferred[Any], Any, AsyncIterable[Any]]: self._check_deprecated_start_requests_use(spider) if self._use_start_requests: sync_seeds = iter(spider.start_requests()) diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index a6ba14697..9ff491944 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -15,7 +15,7 @@ from scrapy.utils.trackref import object_ref from scrapy.utils.url import url_is_from_spider if TYPE_CHECKING: - from collections.abc import AsyncIterator, Iterable + from collections.abc import AsyncIterable, Iterable from twisted.internet.defer import Deferred @@ -78,7 +78,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 yield_seeds(self) -> AsyncIterable[Any]: """Yield the initial :class:`~scrapy.Request` objects to send. .. versionadded:: VERSION diff --git a/scrapy/spiders/init.py b/scrapy/spiders/init.py index 5c84ae5fe..c1d38854b 100644 --- a/scrapy/spiders/init.py +++ b/scrapy/spiders/init.py @@ -1,7 +1,7 @@ from __future__ import annotations import warnings -from collections.abc import AsyncIterator, Iterable +from collections.abc import AsyncIterable, Iterable from typing import TYPE_CHECKING, Any, cast from scrapy import Request @@ -29,7 +29,7 @@ class InitSpider(Spider): stacklevel=2, ) - async def yield_seeds(self) -> AsyncIterator[Any]: + async def yield_seeds(self) -> AsyncIterable[Any]: for seed in self.start_requests(): yield seed diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 1be23421c..bc004f663 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -4,7 +4,7 @@ import logging import re # Iterable is needed at the run time for the SitemapSpider._parse_sitemap() annotation -from collections.abc import AsyncIterator, Iterable, Sequence # noqa: TC003 +from collections.abc import AsyncIterable, Iterable, Sequence # noqa: TC003 from typing import TYPE_CHECKING, Any, cast from scrapy.http import Request, Response, XmlResponse @@ -53,7 +53,7 @@ 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]: + async def yield_seeds(self) -> AsyncIterable[Any]: for seed in self.start_requests(): yield seed From 0ebc0573446a0f48a1e849cbde3bb05f183b7096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Mar 2025 21:21:36 +0100 Subject: [PATCH 03/13] =?UTF-8?q?Replace=20RuntimeError=20handling=20with?= =?UTF-8?q?=20Andrey=E2=80=99s=20lock-based=20approach?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/core/engine.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 7383fc0e0..05f8a266a 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -113,6 +113,7 @@ class ExecutionEngine: self.start_time: float | None = None self._load_seeding_policy() self._seeds: AsyncIterable[Any] | None = None + self._waiting_for_seed: bool = False def _load_seeding_policy(self) -> None: try: @@ -192,15 +193,13 @@ class ExecutionEngine: @inlineCallbacks def _process_next_seed(self): + if self._waiting_for_seed: + return + self._waiting_for_seed = True try: seed = yield deferred_from_coro(self._seeds.__anext__()) except StopAsyncIteration: self._seeds = None - except RuntimeError: - # “RuntimeError: anext(): asynchronous generator is already - # running” happens if yield_seeds is taking long to yield the - # next seed. - pass except Exception: self._seeds = None logger.error( @@ -214,6 +213,8 @@ class ExecutionEngine: else: self.scraper.start_itemproc(seed, response=None) self._slot.nextcall.schedule() + finally: + self._waiting_for_seed = False @inlineCallbacks def _start_next_requests(self) -> Generator[Deferred[Any], Any, None]: From 69f829fa5e267069525ee2313793c84eaa4822db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 00:58:02 +0100 Subject: [PATCH 04/13] Implement seeding policies --- docs/topics/settings.rst | 78 ++++++------- pyproject.toml | 4 +- scrapy/core/engine.py | 46 ++++++-- tests/test_engine_seeding.py | 217 +++++++++++++++++++++++++++++++---- 4 files changed, 268 insertions(+), 77 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index e86c6acbc..042c2ed8a 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1746,60 +1746,52 @@ The way :meth:`Spider.yield_seeds ` is iterated: - .. _lazy-seeding: - ``"lazy"``: Seeds are only read while the :ref:`scheduler - ` is empty and the number of ongoing requests is - lower than :setting:`CONCURRENT_REQUESTS`. + ``"lazy"``: Processing scheduled requests takes priority over iterating + seeds. - This seeding policy aims to: - - - Maximize crawl speed by maxing out concurrent requests as often - as possible. - - - Minimize the number of requests in the scheduler at any given - time by prioritizing scheduler requests over seeds, to minimize - resource usage (memory or disk, depending on - :setting:`JOBDIR`). - - This seeding policy is best used when seed request priority is not - important. Switching to :ref:`serial ` may lower + This seeding policy 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 :ref:`idle ` may lower resource usage further at the cost of also lowering crawl speed. -- .. _front-load-seeding: - - ``"front-load"``: The spider does not start until all seeds have - been read and loaded into the scheduler. - - This seeding policy aims to give the :ref:`scheduler - ` full control over request order, at the cost of - a higher resource usage and a delayed crawl start. - - This seeding policy is best used when having all requests go - through the scheduler is more important than resource usage and - crawl speed. - - .. _greedy-seeding: - ``"greedy"``: While the :ref:`scheduler ` is - empty and the number of ongoing requests is lower than - :setting:`CONCURRENT_REQUESTS`, seeds are read and sent directly - (bypassing the scheduler). While the scheduler has requests, seeds - are fed into the scheduler. + ``"greedy"``: Iterating seeds takes priority over processing scheduled + requests. - This seeding policy is similar to :ref:`front-load - `, but it bypasses the scheduler for the first - few requests to avoid delaying the crawl start. + Every time a seed request is iterated, it is scheduled, and then the next + request from the scheduler is sent. -- .. _serial-seeding: + .. note:: That request sent may not be the schedueld 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`). + + This seeding policy is best used when prioritizing seed requests is + important, and seed requests may be sent as they come. + +- .. _front-load-seeding: + + ``"front-load"``: The spider does not start until all seed requests have + been scheduled. + + This seeding policy 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. + +- .. _idle-seeding: + + ``"idle"``: A single seed is read only when there are neither scheduled nor + on-going requests. - ``"serial"``: A single seed is read whenever the :ref:`scheduler - ` is empty and there are no ongoing requests. That is, a new seed is not read until all requests triggered by the previous seed, directly or indirectly, have been processed. - This seeding policy is similar to :ref:`lazy `, but - it prioritizes resource savings over crawl speed. It is - functionally equivalent to running the spider multiple times in a - row, one per seed request. + This seeding policy is similar to :ref:`lazy `, but it + prioritizes resource savings over crawl speed. It is functionally + equivalent to running your spider multiple times in a row, one per seed + request. .. setting:: SPIDER_CONTRACTS diff --git a/pyproject.toml b/pyproject.toml index 84bf41a94..ed6a6ed46 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -223,7 +223,9 @@ markers = [ "requires_botocore: marks tests that need botocore (but not boto3)", "requires_boto3: marks tests that need botocore and boto3", ] -filterwarnings = [] +filterwarnings = [ + "ignore::DeprecationWarning:twisted.web.static" +] [tool.ruff.lint] extend-select = [ diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 05f8a266a..2b6dc0e81 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -78,10 +78,10 @@ class _Slot: class _SeedingPolicy(Enum): - lazy = "lazy" front_load = "front-load" greedy = "greedy" - serial = "serial" + idle = "idle" + lazy = "lazy" class ExecutionEngine: @@ -186,11 +186,6 @@ class ExecutionEngine: def unpause(self) -> None: self.paused = False - def _start_scheduled_requests(self): - while not self._needs_backout(): - if self._start_scheduled_request() is None: - break - @inlineCallbacks def _process_next_seed(self): if self._waiting_for_seed: @@ -210,19 +205,50 @@ class ExecutionEngine: 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() else: self.scraper.start_itemproc(seed, response=None) 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() @inlineCallbacks def _start_next_requests(self) -> Generator[Deferred[Any], Any, None]: if self._slot is None or self._slot.closing is not None or self.paused: return - self._start_scheduled_requests() - if self._seeds is not None and not self._needs_backout(): - yield self._process_next_seed() + + if self._seeding_policy in {_SeedingPolicy.idle, _SeedingPolicy.lazy}: + while not self._needs_backout(): + if self._start_scheduled_request() is None: + break + if ( + self._seeds is not None + and not self._needs_backout() + and ( + self._seeding_policy is not _SeedingPolicy.idle + or (not self._waiting_for_seed and not self.downloader.active) + ) + ): + yield self._process_next_seed() + else: + assert self._seeding_policy in { + _SeedingPolicy.front_load, + _SeedingPolicy.greedy, + } + if self._seeds is not None: + if not self._needs_backout(): + yield self._process_next_seed() + else: + 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() diff --git a/tests/test_engine_seeding.py b/tests/test_engine_seeding.py index c74dc3ca1..ae9baea63 100644 --- a/tests/test_engine_seeding.py +++ b/tests/test_engine_seeding.py @@ -1,29 +1,36 @@ from __future__ import annotations -from collections import deque +from collections import defaultdict, deque -from twisted.internet.defer import inlineCallbacks from twisted.trial.unittest import TestCase from scrapy import Request, Spider, signals +from scrapy.core.engine import ExecutionEngine from scrapy.core.scheduler import BaseScheduler -from scrapy.utils.defer import maybe_deferred_to_future +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler +from .mockserver import MockServer from .test_spider_yield_seeds import twisted_sleep class MainTestCase(TestCase): - @inlineCallbacks - def test_scheduler_priority_over_seeds_simple(self): - """The seeding policy is to read seeds into the scheduler while the - scheduler is empty, but otherwise priorize requests already in the - scheduler. - - This test shows how, given a scheduler pre-filled with a request, that - request is sent before sending the first seed request. - """ + # If the test ends before the heartbeat, it may mean that the logic to + # re-schecule a new call of _start_next_requests under the right + # ciscumstances is not properly implemented, and the hearatbeat is working + # as a workaround for that issue. This is a performance issue and should + # be addressed. + # + # It could also happen that, on some CI runners, some tests (e.g. those + # below using a mock server) run too slow and proper handling overlaps with + # the heartbeat. If that is the case, it may be worth considering + # increasing the heartbeat time. It should be safe, since in most real live + # scenarios the heartbeat should never make a difference, and we may + # eventually remove the heartbeat altogether. + timeout = ExecutionEngine._SLOT_HEARTBEAT_INTERVAL + @deferred_f_from_coro_f + async def test_lazy(self): class TestScheduler(BaseScheduler): def __init__(self, *args, **kwargs): self.requests = deque((Request("data:,a"),)) @@ -56,20 +63,15 @@ class MainTestCase(TestCase): settings = {"SCHEDULER": TestScheduler} crawler = get_crawler(TestSpider, settings_dict=settings) crawler.signals.connect(track_url, signals.request_reached_downloader) - yield crawler.crawl() + 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=}" - @inlineCallbacks - def test_scheduler_priority_over_seeds_complex(self): - """While the seeding policy is to read seeds into the scheduler while - the scheduler is empty and otherwise priorize requests already in the - scheduler, this is done in a non-blocking way. - - That is, if the scheduler reports having requests but yields none, - requests from seeds will be scheduled. - """ + @deferred_f_from_coro_f + async def test_lazy_blocking(self): + """If the scheduler reports having requests but yields none, the lazy + policy schedules requests from seeds.""" class TestScheduler(BaseScheduler): def __init__(self, *args, **kwargs): @@ -114,7 +116,176 @@ class MainTestCase(TestCase): settings = {"SCHEDULER": TestScheduler} crawler = get_crawler(TestSpider, settings_dict=settings) crawler.signals.connect(track_url, signals.request_reached_downloader) - yield crawler.crawl() + 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_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) + + crawler = get_crawler(TestSpider) + 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_greedy(self): + class TestScheduler(BaseScheduler): + def __init__(self, *args, **kwargs): + self.requests = deque((Request("data:,b"),)) + + def enqueue_request(self, request: Request) -> bool: + self.requests.append(request) + return True + + def has_pending_requests(self) -> bool: + return bool(self.requests) + + def next_request(self) -> Request | None: + try: + return self.requests.pop() + except IndexError: + return None + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,a"] + + def parse(self, response): + pass + + actual_urls = [] + + def track_url(request, spider): + actual_urls.append(request.url) + + settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "greedy"} + 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_front_load(self): + class TestScheduler(BaseScheduler): + def __init__(self, *args, **kwargs): + self.requests = defaultdict(deque) + + def enqueue_request(self, request: Request) -> bool: + self.requests[request.priority].append(request) + return True + + def has_pending_requests(self) -> bool: + return bool(self.requests) + + def next_request(self) -> Request | None: + if not self.requests: + return None + priority = max(self.requests) + request = self.requests[priority].popleft() + if not self.requests[priority]: + del self.requests[priority] + return request + + 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": TestScheduler, "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=}" + + +class MockServerTestCase(TestCase): + timeout = ExecutionEngine._SLOT_HEARTBEAT_INTERVAL + + @classmethod + def setUpClass(cls): + cls.mockserver = MockServer() + cls.mockserver.__enter__() + + @classmethod + def tearDownClass(cls): + cls.mockserver.__exit__(None, None, None) + + @deferred_f_from_coro_f + async def test_idle(self): + def _url(id): + return self.mockserver.url(f"/delay?n=0.1&{id}") + + class TestScheduler(BaseScheduler): + def __init__(self, *args, **kwargs): + self.requests = deque((Request(_url("a")),)) + + def enqueue_request(self, request: Request) -> bool: + self.requests.append(request) + return True + + def has_pending_requests(self) -> bool: + return bool(self.requests) + + def next_request(self) -> Request | None: + try: + return self.requests.popleft() + except IndexError: + return None + + class TestSpider(Spider): + name = "test" + start_urls = [_url("b"), _url("d")] + queue = deque((Request(_url("c")),)) + + def parse(self, response): + try: + request = self.queue.popleft() + except IndexError: + pass + else: + yield request + + actual_urls = [] + + def track_url(request, spider): + actual_urls.append(request.url) + + settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "idle"} + 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 = [_url(letter) for letter in "abcd"] + assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" From 38cf129241c869f590ad7c892fb2d9d96396a160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 02:10:20 +0100 Subject: [PATCH 05/13] Publish the SeedingPolicy enum --- docs/conf.py | 1 + docs/requirements.txt | 1 + docs/topics/settings.rst | 58 ++++++----------------------------- scrapy/__init__.py | 2 ++ scrapy/core/_seeding.py | 65 ++++++++++++++++++++++++++++++++++++++++ scrapy/core/engine.py | 26 +++++++--------- 6 files changed, 88 insertions(+), 65 deletions(-) create mode 100644 scrapy/core/_seeding.py diff --git a/docs/conf.py b/docs/conf.py index 1167ce050..6985e38fe 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -27,6 +27,7 @@ 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/requirements.txt b/docs/requirements.txt index 103fb08d6..63243fcf3 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,3 +1,4 @@ +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/settings.rst b/docs/topics/settings.rst index 042c2ed8a..dd2fb3400 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1740,58 +1740,18 @@ SEEDING_POLICY .. versionadded:: VERSION -Default: ``"lazy"`` +Default: :py:enum:mem:`SeedingPolicy.lazy ` -The way :meth:`Spider.yield_seeds ` is iterated: +Determines the way :meth:`Spider.yield_seeds ` is +iterated. -- .. _lazy-seeding: +Its value may be defined as a member of the :class:`~scrapy.SeedingPolicy` enum +(e.g. :py:enum:mem:`SeedingPolicy.front_load +`) or as the corresponding string (e.g. +``"front-load"``). - ``"lazy"``: Processing scheduled requests takes priority over iterating - seeds. - - This seeding policy 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 :ref:`idle ` may lower - resource usage further at the cost of also lowering crawl speed. - -- .. _greedy-seeding: - - ``"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 schedueld 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`). - - This seeding policy is best used when prioritizing seed requests is - important, and seed requests may be sent as they come. - -- .. _front-load-seeding: - - ``"front-load"``: The spider does not start until all seed requests have - been scheduled. - - This seeding policy 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. - -- .. _idle-seeding: - - ``"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. - - This seeding policy is similar to :ref:`lazy `, but it - prioritizes resource savings over crawl speed. It is functionally - equivalent to running your spider multiple times in a row, one per seed - request. +.. autoenum:: scrapy.SeedingPolicy + :members: .. setting:: SPIDER_CONTRACTS diff --git a/scrapy/__init__.py b/scrapy/__init__.py index 256504c9c..7bb958a2d 100644 --- a/scrapy/__init__.py +++ b/scrapy/__init__.py @@ -7,6 +7,7 @@ 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 @@ -17,6 +18,7 @@ __all__ = [ "FormRequest", "Item", "Request", + "SeedingPolicy", "Selector", "Spider", "__version__", diff --git a/scrapy/core/_seeding.py b/scrapy/core/_seeding.py new file mode 100644 index 000000000..13f5c6d6a --- /dev/null +++ b/scrapy/core/_seeding.py @@ -0,0 +1,65 @@ +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 2b6dc0e81..171b89434 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -8,7 +8,6 @@ For more information see docs/topics/architecture.rst from __future__ import annotations import logging -from enum import Enum from time import time from typing import TYPE_CHECKING, Any, TypeVar, cast @@ -25,6 +24,8 @@ from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.reactor import CallLaterOnce +from ._seeding import SeedingPolicy + if TYPE_CHECKING: from collections.abc import AsyncIterable, Callable, Generator @@ -77,13 +78,6 @@ class _Slot: self.closing.callback(None) -class _SeedingPolicy(Enum): - front_load = "front-load" - greedy = "greedy" - idle = "idle" - lazy = "lazy" - - class ExecutionEngine: _SLOT_HEARTBEAT_INTERVAL: float = 5.0 @@ -117,9 +111,9 @@ class ExecutionEngine: def _load_seeding_policy(self) -> None: try: - self._seeding_policy = _SeedingPolicy(self.settings["SEEDING_POLICY"]) + self._seeding_policy = SeedingPolicy(self.settings["SEEDING_POLICY"]) except ValueError: - supported_values = ", ".join(policy.value for policy in _SeedingPolicy) + 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. " @@ -206,7 +200,7 @@ class ExecutionEngine: if isinstance(seed, Request): self.crawl(seed) if ( - self._seeding_policy is not _SeedingPolicy.front_load + self._seeding_policy is not SeedingPolicy.front_load and not self._needs_backout() ): self._start_scheduled_request() @@ -215,7 +209,7 @@ class ExecutionEngine: self._slot.nextcall.schedule() finally: self._waiting_for_seed = False - if self._seeding_policy is _SeedingPolicy.front_load and self._seeds is None: + if self._seeding_policy is SeedingPolicy.front_load and self._seeds is None: self._slot.nextcall.schedule() @inlineCallbacks @@ -223,7 +217,7 @@ class ExecutionEngine: if self._slot is None or self._slot.closing is not None or self.paused: return - if self._seeding_policy in {_SeedingPolicy.idle, _SeedingPolicy.lazy}: + if self._seeding_policy in {SeedingPolicy.idle, SeedingPolicy.lazy}: while not self._needs_backout(): if self._start_scheduled_request() is None: break @@ -231,15 +225,15 @@ class ExecutionEngine: self._seeds is not None and not self._needs_backout() and ( - self._seeding_policy is not _SeedingPolicy.idle + self._seeding_policy is not SeedingPolicy.idle or (not self._waiting_for_seed and not self.downloader.active) ) ): yield self._process_next_seed() else: assert self._seeding_policy in { - _SeedingPolicy.front_load, - _SeedingPolicy.greedy, + SeedingPolicy.front_load, + SeedingPolicy.greedy, } if self._seeds is not None: if not self._needs_backout(): From e6790ec86b5db8a7a1f9981e98bb164b10110680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 07:31:25 +0100 Subject: [PATCH 06/13] Make the idle seeding policy test more reliable --- tests/test_engine_seeding.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_engine_seeding.py b/tests/test_engine_seeding.py index ae9baea63..4debd90a0 100644 --- a/tests/test_engine_seeding.py +++ b/tests/test_engine_seeding.py @@ -231,7 +231,11 @@ class MainTestCase(TestCase): class MockServerTestCase(TestCase): + # See the comment on the matching line above. timeout = ExecutionEngine._SLOT_HEARTBEAT_INTERVAL + # If requests are too fast, test_idle will fail because the outcome will + # match that of the lazy seeding policy. + delay = 0.2 @classmethod def setUpClass(cls): @@ -245,7 +249,7 @@ class MockServerTestCase(TestCase): @deferred_f_from_coro_f async def test_idle(self): def _url(id): - return self.mockserver.url(f"/delay?n=0.1&{id}") + return self.mockserver.url(f"/delay?n={self.delay}&{id}") class TestScheduler(BaseScheduler): def __init__(self, *args, **kwargs): From a0672532342ae63110557c01cb20b82bc8adc60e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 08:58:12 +0100 Subject: [PATCH 07/13] Allow overriding the active seeding policy --- docs/news.rst | 26 +++++++++++- docs/topics/settings.rst | 9 +++-- docs/topics/spider-middleware.rst | 21 ++++++++-- scrapy/core/_seeding.py | 2 +- scrapy/core/engine.py | 67 +++++++++++++++++++++---------- scrapy/spiders/__init__.py | 13 +++++- tests/test_engine_seeding.py | 66 +++++++++++++++++++++++++++++- 7 files changed, 169 insertions(+), 35 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 7afcbb3cf..d7de9c769 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -40,14 +40,16 @@ Deprecations use :meth:`~scrapy.Spider.yield_seeds` instead, or both to maintain support for lower Scrapy versions. - (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`) + (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6715`, + :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. - (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`) + (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6715`, + :issue:`6729`) New features ~~~~~~~~~~~~ @@ -66,6 +68,26 @@ New features - The new :setting:`SEEDING_POLICY` setting allows customizing how spider start requests and items are consumed. + You can also override the active seeding policy from + :meth:`Spider.yield_seeds ` and from + :meth:`SpiderMiddleware.process_seeds + `. + + .. 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:`740`, :issue:`1051`, :issue:`1443`, :issue:`3237`, :issue:`4467`, + :issue:`5282`, :issue:`6715`) + +Bug fixes +~~~~~~~~~ + +- The first :setting:`CONCURRENT_REQUESTS` start requests are no longer sent + in reserve order by default. + + (:issue:`6715`, :issue:`6729`) + .. _release-2.12.0: diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index dd2fb3400..d773757d0 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1746,9 +1746,12 @@ 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.front_load -`) or as the corresponding string (e.g. -``"front-load"``). +(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: diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 36b440720..9ea46fc53 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -83,12 +83,25 @@ one or more of these methods: async for seed in seeds: yield seed - You may yield :class:`~scrapy.Request` or :ref:`item ` - objects, same as :meth:`~scrapy.Spider.yield_seeds`, from *seeds* or - not. + You may yield the same type of objects as + :meth:`~scrapy.Spider.yield_seeds`. As with :meth:`~scrapy.Spider.yield_seeds`, how this method is iterated - is controlled by :setting:`SEEDING_POLICY`. + 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. To write spider middlewares that work on Scrapy versions lower than VERSION, define also a synchronous ``process_start_requests()`` method diff --git a/scrapy/core/_seeding.py b/scrapy/core/_seeding.py index 13f5c6d6a..83f3f4159 100644 --- a/scrapy/core/_seeding.py +++ b/scrapy/core/_seeding.py @@ -15,7 +15,7 @@ else: @document_enum class SeedingPolicy(Enum): - front_load = "front-load" + 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 diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 171b89434..8383b9395 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -43,6 +43,10 @@ logger = logging.getLogger(__name__) _T = TypeVar("_T") +class _SeedingPolicyChange(Exception): + pass + + class _Slot: def __init__( self, @@ -204,6 +208,21 @@ class ExecutionEngine: 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 else: self.scraper.start_itemproc(seed, response=None) self._slot.nextcall.schedule() @@ -217,31 +236,35 @@ class ExecutionEngine: if self._slot is None or self._slot.closing is not None or self.paused: return - if self._seeding_policy in {SeedingPolicy.idle, SeedingPolicy.lazy}: - while not self._needs_backout(): - if self._start_scheduled_request() is None: - break - if ( - self._seeds is not None - and not self._needs_backout() - and ( - self._seeding_policy is not SeedingPolicy.idle - or (not self._waiting_for_seed and not self.downloader.active) - ) - ): - yield self._process_next_seed() - else: - assert self._seeding_policy in { - SeedingPolicy.front_load, - SeedingPolicy.greedy, - } - if self._seeds is not None: - if not self._needs_backout(): - yield self._process_next_seed() - else: + try: + if self._seeding_policy in {SeedingPolicy.idle, SeedingPolicy.lazy}: while not self._needs_backout(): if self._start_scheduled_request() is None: break + if ( + self._seeds is not None + and not self._needs_backout() + and ( + self._seeding_policy is not SeedingPolicy.idle + or (not self._waiting_for_seed and not self.downloader.active) + ) + ): + yield self._process_next_seed() + else: + assert self._seeding_policy in { + SeedingPolicy.front_load, + SeedingPolicy.greedy, + } + if self._seeds is not None: + if not self._needs_backout(): + yield self._process_next_seed() + else: + while not self._needs_backout(): + if self._start_scheduled_request() is None: + break + except _SeedingPolicyChange: + self._slot.nextcall.schedule() + return if self.spider_is_idle() and self._slot.close_if_idle: self._spider_idle() diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 9ff491944..d2941c7c3 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -114,7 +114,18 @@ class Spider(object_ref): yield {"foo": "bar"} Use :setting:`SEEDING_POLICY` to set how :meth:`yield_seeds` is - iterated. + 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") To write spiders that work on Scrapy versions lower than VERSION, define also a synchronous ``start_requests()`` method that returns an diff --git a/tests/test_engine_seeding.py b/tests/test_engine_seeding.py index 4debd90a0..9fbe4c6e7 100644 --- a/tests/test_engine_seeding.py +++ b/tests/test_engine_seeding.py @@ -1,10 +1,12 @@ from __future__ import annotations from collections import defaultdict, deque +from logging import ERROR +from testfixtures import LogCapture from twisted.trial.unittest import TestCase -from scrapy import Request, Spider, signals +from scrapy import Request, SeedingPolicy, Spider, signals from scrapy.core.engine import ExecutionEngine from scrapy.core.scheduler import BaseScheduler from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future @@ -221,7 +223,7 @@ class MainTestCase(TestCase): def track_url(request, spider): actual_urls.append(request.url) - settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "front-load"} + settings = {"SCHEDULER": TestScheduler, "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()) @@ -229,6 +231,66 @@ class MainTestCase(TestCase): 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 TestScheduler(BaseScheduler): + def __init__(self, *args, **kwargs): + self.requests = defaultdict(deque) + + def enqueue_request(self, request: Request) -> bool: + self.requests[request.priority].append(request) + return True + + def has_pending_requests(self) -> bool: + return bool(self.requests) + + def next_request(self) -> Request | None: + if not self.requests: + return None + priority = max(self.requests) + request = self.requests[priority].popleft() + if not self.requests[priority]: + del self.requests[priority] + return request + + 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": TestScheduler} + 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 actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" + class MockServerTestCase(TestCase): # See the comment on the matching line above. From 57ef3b2689b2b82c65cbe5b479e0615675cf1608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 10:12:03 +0100 Subject: [PATCH 08/13] Update test expectations --- tests/test_crawl.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/test_crawl.py b/tests/test_crawl.py index a45dfec60..61662a702 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -194,15 +194,16 @@ class TestCrawl(TestCase): @defer.inlineCallbacks def test_yield_seeds_unsupported_output(self): - """Anything that is not a request or 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.""" + """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.""" with LogCapture("scrapy", level=logging.ERROR) as log: crawler = get_crawler(YieldSeedsGoodAndBadOutput) yield crawler.crawl(mockserver=self.mockserver) - assert len(log.records) == 0 + assert len(log.records) == 1 @defer.inlineCallbacks def test_yield_seeds_laziness(self): From 724b6a620a4d270d42485c90cc8a47c9581b8074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 11:18:11 +0100 Subject: [PATCH 09/13] news.rst: Remove fake bug fix I was convinced this used to be an issue, but I could not reproduce it in Scrapy 2.12 --- docs/news.rst | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index d7de9c769..dc22b0f69 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -80,14 +80,6 @@ New features (:issue:`740`, :issue:`1051`, :issue:`1443`, :issue:`3237`, :issue:`4467`, :issue:`5282`, :issue:`6715`) -Bug fixes -~~~~~~~~~ - -- The first :setting:`CONCURRENT_REQUESTS` start requests are no longer sent - in reserve order by default. - - (:issue:`6715`, :issue:`6729`) - .. _release-2.12.0: From 1d1a85711d76278dd1fe2b8dac55724f8f278cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 11:23:52 +0100 Subject: [PATCH 10/13] news.rst: cover start-item-related delay fix, reproducible in Scrapy 2.12 --- docs/news.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index dc22b0f69..f3adc3355 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -80,6 +80,15 @@ New features (:issue:`740`, :issue:`1051`, :issue:`1443`, :issue:`3237`, :issue:`4467`, :issue:`5282`, :issue:`6715`) +Bug fixes +~~~~~~~~~ + +- Yielding an 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. + + (:issue:`6715`, :issue:`6729`) + .. _release-2.12.0: From e4e3dba1418a9d55c6078550ebffdf51a8b04d07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 11:25:00 +0100 Subject: [PATCH 11/13] Fix a typo --- docs/news.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/news.rst b/docs/news.rst index f3adc3355..a82d05d67 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -83,7 +83,7 @@ New features Bug fixes ~~~~~~~~~ -- Yielding an start item (i.e. from :meth:`~scrapy.Spider.yield_seeds` or an +- 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. From b1999444ba702e03354f4907e839fcf2e02d3376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 13:17:57 +0100 Subject: [PATCH 12/13] =?UTF-8?q?Default=20seeding=20policy:=20lazy=20?= =?UTF-8?q?=E2=86=92=20greedy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/news.rst | 24 ++++---- docs/topics/settings.rst | 2 +- scrapy/commands/shell.py | 4 +- scrapy/settings/default_settings.py | 4 +- tests/test_crawl.py | 2 +- tests/test_engine_seeding.py | 87 +++++++++++++++-------------- 6 files changed, 66 insertions(+), 57 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index a82d05d67..efe2cd819 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -11,13 +11,16 @@ Scrapy VERSION (unreleased) Highlights: - Replaced ``start_requests`` (sync) with :meth:`~scrapy.Spider.yield_seeds` - (async) + (async) and changed how it is iterated by default. Backward-incompatible changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- In ``scrapy.core.spidermw.SpiderMiddlewareManager``, - ``process_start_requests()`` has been replaced by ``process_seeds()``. +- By default, the iteration of start requests and items no longer stops once + there are requests in the scheduler. + + You can restore the previous behavior by setting :setting:`SEEDING_POLICY` + to :py:enum:mem:`~scrapy.SeedingPolicy.lazy`. - In ``scrapy.core.engine.ExecutionEngine``: @@ -33,6 +36,9 @@ 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()``. + Deprecations ~~~~~~~~~~~~ @@ -40,16 +46,14 @@ Deprecations use :meth:`~scrapy.Spider.yield_seeds` instead, or both to maintain support for lower Scrapy versions. - (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6715`, - :issue:`6729`) + (: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. - (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6715`, - :issue:`6729`) + (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) New features ~~~~~~~~~~~~ @@ -65,8 +69,8 @@ New features (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`) -- The new :setting:`SEEDING_POLICY` setting allows customizing how spider - start requests and items are consumed. +- The new :setting:`SEEDING_POLICY` setting allows customizing how start + requests and items are iterated. You can also override the active seeding policy from :meth:`Spider.yield_seeds ` and from @@ -87,7 +91,7 @@ Bug fixes equivalent) no longer delays the next iteration of starting requests and items by up to 5 seconds. - (:issue:`6715`, :issue:`6729`) + (:issue:`6729`) .. _release-2.12.0: diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index d773757d0..7aa340d8f 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1740,7 +1740,7 @@ SEEDING_POLICY .. versionadded:: VERSION -Default: :py:enum:mem:`SeedingPolicy.lazy ` +Default: :py:enum:mem:`SeedingPolicy.greedy ` Determines the way :meth:`Spider.yield_seeds ` is iterated. diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 3047ae396..c50c963ef 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -9,6 +9,7 @@ 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 @@ -24,9 +25,10 @@ if TYPE_CHECKING: class Command(ScrapyCommand): requires_project = False default_settings = { + "DUPEFILTER_CLASS": "scrapy.dupefilters.BaseDupeFilter", "KEEP_ALIVE": True, "LOGSTATS_INTERVAL": 0, - "DUPEFILTER_CLASS": "scrapy.dupefilters.BaseDupeFilter", + "SEEDING_POLICY": SeedingPolicy.lazy, } def syntax(self) -> str: diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index a08becfee..886154bfa 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -17,6 +17,8 @@ import sys from importlib import import_module from pathlib import Path +from scrapy import SeedingPolicy + ADDONS = {} AJAXCRAWL_ENABLED = False @@ -308,7 +310,7 @@ SCHEDULER_PRIORITY_QUEUE = "scrapy.pqueues.ScrapyPriorityQueue" SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5000000 -SEEDING_POLICY = "lazy" +SEEDING_POLICY = SeedingPolicy.greedy SPIDER_LOADER_CLASS = "scrapy.spiderloader.SpiderLoader" SPIDER_LOADER_WARN_ONLY = False diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 61662a702..442f04086 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -207,7 +207,7 @@ class TestCrawl(TestCase): @defer.inlineCallbacks def test_yield_seeds_laziness(self): - settings = {"CONCURRENT_REQUESTS": 1} + settings = {"CONCURRENT_REQUESTS": 1, "SEEDING_POLICY": "lazy"} crawler = get_crawler(BrokenYieldSeedsSpider, settings) yield crawler.crawl(mockserver=self.mockserver) assert crawler.spider.seedsseen.index(None) < crawler.spider.seedsseen.index( diff --git a/tests/test_engine_seeding.py b/tests/test_engine_seeding.py index 9fbe4c6e7..c538fae44 100644 --- a/tests/test_engine_seeding.py +++ b/tests/test_engine_seeding.py @@ -31,6 +31,45 @@ class MainTestCase(TestCase): # eventually remove the heartbeat altogether. timeout = ExecutionEngine._SLOT_HEARTBEAT_INTERVAL + @deferred_f_from_coro_f + async def test_greedy(self): + class TestScheduler(BaseScheduler): + def __init__(self, *args, **kwargs): + self.requests = deque((Request("data:,b"),)) + + def enqueue_request(self, request: Request) -> bool: + self.requests.append(request) + return True + + def has_pending_requests(self) -> bool: + return bool(self.requests) + + def next_request(self) -> Request | None: + try: + return self.requests.pop() + except IndexError: + return None + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,a"] + + 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) + 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): class TestScheduler(BaseScheduler): @@ -62,7 +101,7 @@ class MainTestCase(TestCase): def track_url(request, spider): actual_urls.append(request.url) - settings = {"SCHEDULER": TestScheduler} + 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()) @@ -115,7 +154,7 @@ class MainTestCase(TestCase): def track_url(request, spider): actual_urls.append(request.url) - settings = {"SCHEDULER": TestScheduler} + 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()) @@ -140,50 +179,12 @@ class MainTestCase(TestCase): def track_url(request, spider): actual_urls.append(request.url) - crawler = get_crawler(TestSpider) - 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_greedy(self): - class TestScheduler(BaseScheduler): - def __init__(self, *args, **kwargs): - self.requests = deque((Request("data:,b"),)) - - def enqueue_request(self, request: Request) -> bool: - self.requests.append(request) - return True - - def has_pending_requests(self) -> bool: - return bool(self.requests) - - def next_request(self) -> Request | None: - try: - return self.requests.pop() - except IndexError: - return None - - class TestSpider(Spider): - name = "test" - start_urls = ["data:,a"] - - def parse(self, response): - pass - - actual_urls = [] - - def track_url(request, spider): - actual_urls.append(request.url) - - settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "greedy"} + 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"] + expected_urls = ["data:,a", "data:,b", "data:,c"] assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" @deferred_f_from_coro_f @@ -276,7 +277,7 @@ class MainTestCase(TestCase): def track_url(request, spider): actual_urls.append(request.url) - settings = {"SCHEDULER": TestScheduler} + settings = {"SCHEDULER": TestScheduler, "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) From b56a883d58bcf74d4324e73ce12c5e323263f252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 13 Mar 2025 14:07:49 +0100 Subject: [PATCH 13/13] news.rst: Update related GitHub issues --- docs/news.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index efe2cd819..7e8f59b1a 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -67,7 +67,7 @@ New features requests and items, e.g. reading them from a queue service or database using an asynchronous client, without workarounds. - (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`) + (: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. @@ -82,7 +82,7 @@ New features ability to override the active seeding policy. (:issue:`740`, :issue:`1051`, :issue:`1443`, :issue:`3237`, :issue:`4467`, - :issue:`5282`, :issue:`6715`) + :issue:`5282`, :issue:`6730`) Bug fixes ~~~~~~~~~