From b2d4eedea8873700b2597a44dabafe7c9d169275 Mon Sep 17 00:00:00 2001 From: Fandu <113630375+mrfandu1@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:15:03 +0545 Subject: [PATCH 1/7] Fix immediate delivery of full feed export batches (#7730) (#7733) * Store full feed batches before spider closes (#7730) Start closing and storing each batch as soon as it reaches the configured item count. Track unfinished close tasks so spider shutdown still waits for all deliveries before emitting the exporter-closed signal. Add an end-to-end regression test that verifies the first batch is stored while the crawl is still running. * Remove the issue reference --------- Co-authored-by: Andrey Rakhmatullin --- scrapy/extensions/feedexport.py | 39 +++++++++++++++++++++++-------- tests/test_feedexport_batch.py | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 678a29e2e..c2997921d 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -13,7 +13,7 @@ import re import sys import warnings from abc import ABC, abstractmethod -from collections.abc import Callable, Coroutine +from collections.abc import Callable from datetime import datetime, timezone from pathlib import Path, PureWindowsPath from tempfile import NamedTemporaryFile @@ -475,7 +475,7 @@ class FeedExporter: self.feeds = {} self.slots: list[FeedSlot] = [] self.filters: dict[str, ItemFilter] = {} - self._pending_close_coros: list[Coroutine[Any, Any, None]] = [] + self._pending_close_tasks: list[asyncio.Task[None] | Deferred[None]] = [] if not self.settings["FEEDS"] and not self.settings["FEED_URI"]: raise NotConfigured @@ -539,23 +539,44 @@ class FeedExporter: ) async def close_spider(self, spider: Spider) -> None: - self._pending_close_coros.extend( - self._close_slot(slot, spider) for slot in self.slots - ) + for slot in self.slots: + self._schedule_slot_close(slot, spider) - if self._pending_close_coros: + if self._pending_close_tasks: if is_asyncio_available(): await asyncio.wait( - [asyncio.create_task(coro) for coro in self._pending_close_coros] + cast("list[asyncio.Task[None]]", list(self._pending_close_tasks)) ) else: await DeferredList( - deferred_from_coro(coro) for coro in self._pending_close_coros + cast("list[Deferred[None]]", list(self._pending_close_tasks)) ) # Send FEED_EXPORTER_CLOSED signal await self.crawler.signals.send_catch_log_async(signals.feed_exporter_closed) + def _schedule_slot_close( + self, slot: FeedSlot, spider: Spider + ) -> asyncio.Task[None] | Deferred[None]: + """Start closing the slot without waiting for it to finish, keeping + track of the pending work so that it can be awaited in + :meth:`close_spider` if it hasn't finished by then.""" + aw: asyncio.Task[None] | Deferred[None] + coro = self._close_slot(slot, spider) + if is_asyncio_available(): + aw = asyncio.create_task(coro) + self._pending_close_tasks.append(aw) + aw.add_done_callback(self._pending_close_tasks.remove) + else: + aw = deferred_from_coro(coro) + self._pending_close_tasks.append(aw) + aw.addBoth(self._untrack_pending_close_task, aw) + return aw + + def _untrack_pending_close_task(self, result: Any, aw: Deferred[None]) -> Any: + self._pending_close_tasks.remove(aw) + return result + @staticmethod def _get_file(slot_: FeedSlot) -> IO[bytes]: assert slot_.file @@ -652,7 +673,7 @@ class FeedExporter: uri_params = self._get_uri_params( spider, self.feeds[slot.uri_template]["uri_params"], slot ) - self._pending_close_coros.append(self._close_slot(slot, spider)) + self._schedule_slot_close(slot, spider) slots.append( self._start_new_batch( batch_id=slot.batch_id + 1, diff --git a/tests/test_feedexport_batch.py b/tests/test_feedexport_batch.py index 80ff6229b..4b0962c43 100644 --- a/tests/test_feedexport_batch.py +++ b/tests/test_feedexport_batch.py @@ -210,6 +210,47 @@ class TestBatchDeliveries(TestFeedExportBase): header = MyItem.fields.keys() await self.assertExported(items, header, rows, settings=settings) + @coroutine_test + async def test_batch_delivered_when_full(self): + """Full batches must be finalized and delivered as soon as they are + full, instead of when the spider closes.""" + dir_path = self._random_temp_filename() + batch1_path = Path(dir_path, "1.json") + mockserver_url = self.mockserver.url("/") + batch1_contents: list[bytes | None] = [] + + class TestSpider(scrapy.Spider): + name = "testspider" + start_urls = [mockserver_url] + + def parse(self, response): + yield {"foo": "bar1"} + yield {"foo": "bar2"} + yield scrapy.Request( + mockserver_url, callback=self.parse2, dont_filter=True + ) + + def parse2(self, response): + # the first batch was full after the second item, so it must + # have been delivered by now + batch1_contents.append( + batch1_path.read_bytes() if batch1_path.exists() else None + ) + yield {"foo": "bar3"} + + settings = { + "FEEDS": { + build_url(dir_path / "%(batch_id)d.json"): {"format": "json"}, + }, + "FEED_EXPORT_BATCH_ITEM_COUNT": 2, + } + crawler = get_crawler(TestSpider, settings) + await crawler.crawl_async() + + assert batch1_contents, "the second request was not processed" + assert batch1_contents[0] is not None, "batch 1 was not stored during the crawl" + assert json.loads(batch1_contents[0]) == [{"foo": "bar1"}, {"foo": "bar2"}] + def test_wrong_path(self): """If path is without %(batch_time)s and %(batch_id) an exception must be raised""" settings = { From 5b4828a012fcd136a8f46915e19be07a5029e57e Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 30 Jul 2026 03:02:54 -0300 Subject: [PATCH 2/7] docs(practices): Remove scrapoxy mention (#7817) --- docs/topics/practices.rst | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 23738c98c..dfa1e21f6 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -533,8 +533,7 @@ Here are some tips to keep in mind when dealing with these kinds of sites: * if possible, use `Common Crawl`_ to fetch pages, instead of hitting the sites directly * use a pool of rotating IPs. For example, the free `Tor project`_ or paid - services like `ProxyMesh`_. An open source alternative is `scrapoxy`_, a - super proxy that you can attach your own proxies to. + services like `ProxyMesh`_. * for HTTPS websites, if blocking appears related to TLS behavior, consider adjusting the :setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` settings, since some websites may respond @@ -559,5 +558,4 @@ projects that detects common mistakes and anti-patterns. .. _ProxyMesh: https://proxymesh.com/ .. _Common Crawl: https://commoncrawl.org/ .. _testspiders: https://github.com/scrapinghub/testspiders -.. _scrapoxy: https://scrapoxy.io/ .. _Zyte API: https://docs.zyte.com/zyte-api/get-started.html From 98696efa809ddde93f78cabd555db832f270553f Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 12:46:45 +0200 Subject: [PATCH 3/7] Export item fields in declaration order (#7824) --- docs/topics/exporters.rst | 10 ++++++++++ scrapy/exporters.py | 17 ++++++++++++++++- tests/test_exporters.py | 12 ++++++++++++ tests/test_feedexport.py | 12 ++++++------ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index ecd154122..c43b7e20f 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -211,6 +211,16 @@ BaseItemExporter - ``None`` (all fields [2]_, default) + Fields are exported in declaration order, i.e. the order in which + they are defined in the :ref:`item class `. For + :class:`dict` items, which have no declared fields, the key order of + each item is used instead. + + .. versionchanged:: VERSION + Fields of non-\ :class:`dict` items used to be exported in the + order in which they had been populated, except in + :class:`CsvItemExporter`, which has always used declaration order. + - A list of fields: .. code-block:: python diff --git a/scrapy/exporters.py b/scrapy/exporters.py index 5a11df833..ea600d1a8 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -74,6 +74,17 @@ class BaseItemExporter(ABC): def finish_exporting(self) -> None: # noqa: B027 pass + @staticmethod + def _get_populated_field_names(adapter: ItemAdapter) -> Iterable[str]: + """Return the populated field names of *adapter*, in declaration order. + + Populated fields that are not declared, which some item types allow, + come last, in item order. + """ + populated = set(adapter.keys()) + declared = (name for name in adapter.field_names() if name in populated) + return dict.fromkeys([*declared, *adapter.keys()]) + def _get_serialized_fields( self, item: Any, default_value: Any = None, include_empty: bool | None = None ) -> Iterable[tuple[str, Any]]: @@ -86,7 +97,11 @@ class BaseItemExporter(ABC): include_empty = self.export_empty_fields if self.fields_to_export is None: - field_iter = item.field_names() if include_empty else item.keys() + field_iter = ( + item.field_names() + if include_empty + else self._get_populated_field_names(item) + ) elif isinstance(self.fields_to_export, Mapping): if include_empty: field_iter = self.fields_to_export.items() diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 3b997767e..b857728ba 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -118,6 +118,18 @@ class TestBaseItemExporter(ABC): ie = self._get_exporter(fields_to_export={"name": "名稱"}) assert list(ie._get_serialized_fields(self.i)) == [("名稱", "John\xa3")] + def test_field_order(self): + item = self.item_class(age="22", name="John\xa3") + ie = self._get_exporter() + assert [name for name, _ in ie._get_serialized_fields(item)] == ["name", "age"] + + def test_field_order_dict_item(self): + ie = self._get_exporter() + assert [name for name, _ in ie._get_serialized_fields({"age": "22"})] == ["age"] + assert [ + name for name, _ in ie._get_serialized_fields({"age": "22", "name": "John"}) + ] == ["age", "name"] + def test_field_custom_serializer(self): i = self.custom_field_item_class(name="John\xa3", age="22") a = ItemAdapter(i) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 92c414bef..1cca287af 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -675,14 +675,14 @@ class TestFeedExport(TestFeedExportBase): formats = { "csv": b"foo,egg,baz\r\nbar1,spam1,\r\n", - "json": b'[\n{"hello": "world2", "foo": "bar2"}\n]', + "json": b'[\n{"foo": "bar2", "hello": "world2"}\n]', "jsonlines": ( - b'{"foo": "bar1", "egg": "spam1"}\n{"hello": "world2", "foo": "bar2"}\n' + b'{"foo": "bar1", "egg": "spam1"}\n{"foo": "bar2", "hello": "world2"}\n' ), "xml": ( b'\n\n' - b"bar1spam1\n" - b"world2bar2\nworld3" + b"bar1spam1\n" + b"bar2world2\nworld3" b"spam3\n" ), } @@ -740,8 +740,8 @@ class TestFeedExport(TestFeedExportBase): "json": b'[\n{"foo": "bar1", "egg": "spam1"}\n]', "xml": ( b'\n\n' - b"bar1spam1\n" - b"world2bar2\n" + b"bar1spam1\n" + b"bar2world2\n" ), "jsonlines": b'{"foo": "bar1", "egg": "spam1"}\n', } From 433603e6cab4ccea6aee18843deaa03f61e216b8 Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 15:51:45 +0200 Subject: [PATCH 4/7] Add AWS_MAX_POOL_CONNECTIONS (#7794) --- docs/topics/feed-exports.rst | 5 +++-- docs/topics/media-pipeline.rst | 3 +++ docs/topics/settings.rst | 20 +++++++++++++++++ scrapy/extensions/feedexport.py | 11 +++++++++ scrapy/pipelines/files.py | 13 ++++++++++- scrapy/settings/default_settings.py | 2 ++ scrapy/utils/boto.py | 15 +++++++++++++ tests/test_feedexport_storages.py | 35 +++++++++++++++++++++++++++++ tests/test_pipeline_files.py | 27 ++++++++++++++++++++++ 9 files changed, 128 insertions(+), 3 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 66768c97b..2f686fd0f 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -218,12 +218,13 @@ passed through the following settings: .. _temporary security credentials: https://docs.aws.amazon.com/IAM/latest/UserGuide/security-creds.html -You can also define a custom ACL, custom endpoint, and region name for exported -feeds using these settings: +You can also define a custom ACL, custom endpoint, region name and connection +pool size for exported feeds using these settings: - :setting:`FEED_STORAGE_S3_ACL` - :setting:`AWS_ENDPOINT_URL` - :setting:`AWS_REGION_NAME` +- :setting:`AWS_MAX_POOL_CONNECTIONS` The default value for the ``overwrite`` key in the :setting:`FEEDS` for this storage backend is: ``True``. diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 4ceb4732a..b16066d0c 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -268,6 +268,9 @@ For self-hosting you also might feel the need not to use SSL and not to verify S AWS_USE_SSL = False # or True (None by default) AWS_VERIFY = False # or True (None by default) +To reuse connections for as many files as you check or upload in parallel, set +:setting:`AWS_MAX_POOL_CONNECTIONS` accordingly. + .. _canned ACLs: https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html#canned-acl .. _Minio: https://github.com/minio/minio .. _Zenko CloudServer: https://www.zenko.io/cloudserver/ diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 8068824e3..1b6851d04 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -458,6 +458,26 @@ Default: ``None`` Endpoint URL used for S3-like storage, for example Minio or s3.scality. +.. setting:: AWS_MAX_POOL_CONNECTIONS + +AWS_MAX_POOL_CONNECTIONS +------------------------ + +.. versionadded:: VERSION + +Default: ``None`` + +Maximum number of connections that AWS clients, such as those of the +:ref:`S3 feed storage backend ` and of the +:ref:`S3 media pipeline storage backend `, keep in their +connection pool. + +If ``None``, the value of :setting:`REACTOR_THREADPOOL_MAXSIZE` is used. + +Values lower than the number of parallel AWS calls do not limit those calls, but +their connections are closed instead of reused, which hurts performance, and +``Connection pool is full, discarding connection`` warnings are logged. + .. setting:: AWS_REGION_NAME AWS_REGION_NAME diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index c2997921d..448279546 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -28,6 +28,7 @@ from scrapy import Spider, signals from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.extensions.postprocessing import PostProcessingManager from scrapy.utils.asyncio import is_asyncio_available, run_in_thread +from scrapy.utils.boto import _get_max_pool_connections from scrapy.utils.conf import feed_complete_default_values_from_settings from scrapy.utils.defer import deferred_from_coro, ensure_awaitable from scrapy.utils.ftp import ftp_store_file @@ -213,11 +214,14 @@ class S3FeedStorage(BlockingFeedStorage): feed_options: dict[str, Any] | None = None, session_token: str | None = None, region_name: str | None = None, + max_pool_connections: int | None = None, ): try: import boto3.session # noqa: PLC0415 except ImportError: raise NotConfigured("missing boto3 library") from None + from botocore.config import Config # noqa: PLC0415 + u = urlparse(uri) assert u.hostname self.bucketname: str = u.hostname @@ -228,6 +232,7 @@ class S3FeedStorage(BlockingFeedStorage): self.acl: str | None = acl self.endpoint_url: str | None = endpoint_url self.region_name: str | None = region_name + self.max_pool_connections: int | None = max_pool_connections boto3_session = boto3.session.Session() self.s3_client = boto3_session.client( @@ -237,6 +242,11 @@ class S3FeedStorage(BlockingFeedStorage): aws_session_token=self.session_token, endpoint_url=self.endpoint_url, region_name=self.region_name, + config=( + Config(max_pool_connections=self.max_pool_connections) + if self.max_pool_connections is not None + else None + ), ) if feed_options and feed_options.get("overwrite", True) is False: @@ -262,6 +272,7 @@ class S3FeedStorage(BlockingFeedStorage): acl=crawler.settings["FEED_STORAGE_S3_ACL"] or None, endpoint_url=crawler.settings["AWS_ENDPOINT_URL"] or None, region_name=crawler.settings["AWS_REGION_NAME"] or None, + max_pool_connections=_get_max_pool_connections(crawler.settings), feed_options=feed_options, ) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 8e8082332..55a3676e5 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -37,7 +37,7 @@ from scrapy.pipelines.media import ( _MediaRequestFiltered, ) from scrapy.utils.asyncio import run_in_thread -from scrapy.utils.boto import is_botocore_available +from scrapy.utils.boto import _get_max_pool_connections, is_botocore_available from scrapy.utils.datatypes import CaseInsensitiveDict from scrapy.utils.defer import deferred_from_coro, ensure_awaitable from scrapy.utils.ftp import ftp_store_file @@ -164,6 +164,9 @@ class S3FilesStore: AWS_REGION_NAME = None AWS_USE_SSL = None AWS_VERIFY = None + # Overridden from settings.AWS_MAX_POOL_CONNECTIONS in + # FilesPipeline.from_crawler(); None means the botocore default + AWS_MAX_POOL_CONNECTIONS: int | None = None POLICY = "private" # Overridden from settings.FILES_STORE_S3_ACL in FilesPipeline.from_crawler() HEADERS: ClassVar[dict[str, str]] = { @@ -174,7 +177,13 @@ class S3FilesStore: if not is_botocore_available(): raise NotConfigured("missing botocore library") import botocore.session # noqa: PLC0415 + from botocore.config import Config # noqa: PLC0415 + config = ( + Config(max_pool_connections=self.AWS_MAX_POOL_CONNECTIONS) + if self.AWS_MAX_POOL_CONNECTIONS is not None + else None + ) session = botocore.session.get_session() self.s3_client = session.create_client( "s3", @@ -185,6 +194,7 @@ class S3FilesStore: region_name=self.AWS_REGION_NAME, use_ssl=self.AWS_USE_SSL, verify=self.AWS_VERIFY, + config=config, ) if not uri.startswith("s3://"): raise ValueError(f"Incorrect URI scheme in {uri}, expected 's3'") @@ -522,6 +532,7 @@ class FilesPipeline(MediaPipeline): s3store.AWS_REGION_NAME = settings["AWS_REGION_NAME"] s3store.AWS_USE_SSL = settings["AWS_USE_SSL"] s3store.AWS_VERIFY = settings["AWS_VERIFY"] + s3store.AWS_MAX_POOL_CONNECTIONS = _get_max_pool_connections(settings) s3store.POLICY = settings["FILES_STORE_S3_ACL"] gcs_store: type[GCSFilesStore] = cast( diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 993f2436d..a44b36c8a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -28,6 +28,7 @@ __all__ = [ "AUTOTHROTTLE_TARGET_CONCURRENCY", "AWS_ACCESS_KEY_ID", "AWS_ENDPOINT_URL", + "AWS_MAX_POOL_CONNECTIONS", "AWS_REGION_NAME", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN", @@ -229,6 +230,7 @@ AUTOTHROTTLE_TARGET_CONCURRENCY = 1.0 AWS_ACCESS_KEY_ID = None AWS_SECRET_ACCESS_KEY = None AWS_ENDPOINT_URL = None +AWS_MAX_POOL_CONNECTIONS = None AWS_REGION_NAME = None AWS_SESSION_TOKEN = None AWS_USE_SSL = None diff --git a/scrapy/utils/boto.py b/scrapy/utils/boto.py index 2a77ee2ac..76ee0e7ec 100644 --- a/scrapy/utils/boto.py +++ b/scrapy/utils/boto.py @@ -1,7 +1,22 @@ """Boto/botocore helpers""" +from __future__ import annotations + from importlib.util import find_spec +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from scrapy.settings import BaseSettings def is_botocore_available() -> bool: return find_spec("botocore") is not None + + +def _get_max_pool_connections(settings: BaseSettings) -> int: + """Return the maximum number of connections that AWS clients may keep in + their connection pool. + """ + return settings.getint("AWS_MAX_POOL_CONNECTIONS") or settings.getint( + "REACTOR_THREADPOOL_MAXSIZE" + ) diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index 66488540f..b1e3787fd 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -381,6 +381,41 @@ class TestS3FeedStorage: assert storage.region_name == region_name assert storage.s3_client._client_config.region_name == region_name + def test_init_without_max_pool_connections(self) -> None: + storage = S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") + assert storage.max_pool_connections is None + config: Any = storage.s3_client.meta.config + assert config.max_pool_connections == 10 + + def test_init_with_max_pool_connections(self) -> None: + storage = S3FeedStorage( + "s3://mybucket/export.csv", + "access_key", + "secret_key", + max_pool_connections=30, + ) + assert storage.max_pool_connections == 30 + config: Any = storage.s3_client.meta.config + assert config.max_pool_connections == 30 + + @pytest.mark.parametrize( + ("settings", "expected"), + [ + ({}, 10), + ({"REACTOR_THREADPOOL_MAXSIZE": 20}, 20), + ({"AWS_MAX_POOL_CONNECTIONS": 30}, 30), + ({"AWS_MAX_POOL_CONNECTIONS": 30, "REACTOR_THREADPOOL_MAXSIZE": 20}, 30), + ], + ) + def test_from_crawler_max_pool_connections( + self, settings: dict[str, Any], expected: int + ) -> None: + crawler = get_crawler(settings_dict=settings) + storage = S3FeedStorage.from_crawler(crawler, "s3://mybucket/export.csv") + assert storage.max_pool_connections == expected + config: Any = storage.s3_client.meta.config + assert config.max_pool_connections == expected + @coroutine_test async def test_store_without_acl(self): storage = S3FeedStorage( diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 97daa514e..4e7fb118b 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -895,6 +895,33 @@ class TestS3FilesStore: stub.assert_no_pending_responses() + def test_default_max_pool_connections(self) -> None: + store = S3FilesStore("s3://mybucket/prefix/") + config: Any = store.s3_client.meta.config + assert config.max_pool_connections == 10 + + @pytest.mark.parametrize( + ("settings", "expected"), + [ + ({}, 10), + ({"REACTOR_THREADPOOL_MAXSIZE": 20}, 20), + ({"AWS_MAX_POOL_CONNECTIONS": 30}, 30), + ({"AWS_MAX_POOL_CONNECTIONS": 30, "REACTOR_THREADPOOL_MAXSIZE": 20}, 30), + ], + ) + def test_max_pool_connections( + self, monkeypatch: pytest.MonkeyPatch, settings: dict[str, Any], expected: int + ) -> None: + # restores the value that FilesPipeline.from_crawler() sets on the class + monkeypatch.setattr(S3FilesStore, "AWS_MAX_POOL_CONNECTIONS", None) + crawler = get_crawler( + settings_dict={"FILES_STORE": "s3://mybucket/prefix/", **settings} + ) + store = FilesPipeline.from_crawler(crawler).store + assert isinstance(store, S3FilesStore) + config: Any = store.s3_client.meta.config + assert config.max_pool_connections == expected + class TestGCSFilesStore: @staticmethod From f02a99fe71dd0ff2fde1ef7a533cd61904fa1c9f Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 17:15:15 +0200 Subject: [PATCH 5/7] Add doc sections for callbacks and errbacks (#7821) --- docs/faq.rst | 2 +- docs/intro/tutorial.rst | 2 +- docs/topics/coroutines.rst | 4 +- docs/topics/jobs.rst | 10 +- docs/topics/request-response.rst | 480 ++++++++++++++++++++----------- docs/topics/spiders.rst | 67 ++--- scrapy/http/request/__init__.py | 8 +- scrapy/spiders/__init__.py | 16 ++ 8 files changed, 372 insertions(+), 217 deletions(-) diff --git a/docs/faq.rst b/docs/faq.rst index 0446a6868..1a574e5da 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -97,7 +97,7 @@ handler documentation. How can I scrape an item with attributes in different pages? ------------------------------------------------------------ -See :ref:`topics-request-response-ref-request-callback-arguments`. +See :ref:`callback-data`. How can I simulate a user login in my spider? --------------------------------------------- diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index c4e04364b..eaf492c95 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -769,7 +769,7 @@ crawlers on top of it. Also, a common pattern is to build an item with data from more than one page, using a :ref:`trick to pass additional data to the callbacks -`. +`. Using spider arguments diff --git a/docs/topics/coroutines.rst b/docs/topics/coroutines.rst index 9dcd9d69c..b7ddb0a57 100644 --- a/docs/topics/coroutines.rst +++ b/docs/topics/coroutines.rst @@ -21,7 +21,9 @@ hence use coroutine syntax (e.g. ``await``, ``async for``, ``async with``): .. versionadded:: 2.13 -- :class:`~scrapy.Request` callbacks. +- :class:`~scrapy.Request` :ref:`callbacks `, which may + also be defined as :term:`asynchronous generators `. - The :meth:`process_item` method of :ref:`item pipelines `. diff --git a/docs/topics/jobs.rst b/docs/topics/jobs.rst index c9916110d..dcff10772 100644 --- a/docs/topics/jobs.rst +++ b/docs/topics/jobs.rst @@ -96,9 +96,13 @@ Request serialization --------------------- For persistence to work, :class:`~scrapy.Request` objects must be -serializable with :mod:`pickle`, except for the ``callback`` and ``errback`` -values passed to their ``__init__`` method, which must be methods of the -running :class:`~scrapy.Spider` class. +serializable with :mod:`pickle`, except for the :ref:`callback +` and :ref:`errback +` values passed to their ``__init__`` +method, which must be methods of the running :class:`~scrapy.Spider` class. + +Requests that cannot be serialized are kept in memory only: they are still +sent, but they are lost when the crawl is paused. If you wish to log the requests that couldn't be serialized, you can set the :setting:`SCHEDULER_DEBUG` setting to ``True`` in the project's settings page. diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 8e565907f..75158440b 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -205,10 +205,11 @@ Request objects Request metadata can also be accessed through the :attr:`~scrapy.http.Response.meta` attribute of a response. - To pass data from one spider callback to another, consider using - :attr:`cb_kwargs` instead. However, request metadata may be the right - choice in certain scenarios, such as to maintain some debugging data - across all follow-up requests (e.g. the source URL). + To pass your own data from one spider callback to another, use + :attr:`cb_kwargs` instead, see :ref:`callback-data`. However, request + metadata may be the right choice in certain scenarios, such as to + maintain some debugging data across all follow-up requests (e.g. the + source URL). A common use of request metadata is to define request-specific parameters for Scrapy components (extensions, middlewares, etc.). For @@ -248,7 +249,7 @@ Request objects .. method:: Request.copy() Return a new Request which is a copy of this Request. See also: - :ref:`topics-request-response-ref-request-callback-arguments`. + :ref:`callback-data`. .. method:: Request.replace([url, method, headers, body, cookies, meta, flags, encoding, priority, dont_filter, callback, errback, cb_kwargs, cls]) @@ -256,7 +257,7 @@ Request objects given new values by whichever keyword arguments are specified. The :attr:`~scrapy.Request.cb_kwargs` and :attr:`~scrapy.Request.meta` attributes are shallow copied by default (unless new values are given as arguments). See also - :ref:`topics-request-response-ref-request-callback-arguments`. + :ref:`callback-data`. .. automethod:: from_curl @@ -347,160 +348,6 @@ Other functions related to requests .. autofunction:: scrapy.utils.httpobj.urlparse_cached -.. _topics-request-response-ref-request-callback-arguments: - -Passing additional data to callback functions ---------------------------------------------- - -The callback of a request is a function that will be called when the response -of that request is downloaded. The callback function will be called with the -downloaded :class:`Response` object as its first argument. - -Example: - -.. code-block:: python - - def parse_page1(self, response): - return scrapy.Request( - "http://www.example.com/some_page.html", callback=self.parse_page2 - ) - - - def parse_page2(self, response): - # this would log http://www.example.com/some_page.html - self.logger.info("Visited %s", response.url) - -In some cases you may be interested in passing arguments to those callback -functions so you can receive the arguments later, in the second callback. -The following example shows how to achieve this by using the -:attr:`.Request.cb_kwargs` attribute: - -.. code-block:: python - - def parse(self, response): - request = scrapy.Request( - "http://www.example.com/index.html", - callback=self.parse_page2, - cb_kwargs=dict(main_url=response.url), - ) - request.cb_kwargs["foo"] = "bar" # add more arguments for the callback - yield request - - - def parse_page2(self, response, main_url, foo): - yield dict( - main_url=main_url, - other_url=response.url, - foo=foo, - ) - -.. caution:: :attr:`.Request.cb_kwargs` was introduced in version ``1.7``. - Prior to that, using :attr:`.Request.meta` was recommended for passing - information around callbacks. After ``1.7``, :attr:`.Request.cb_kwargs` - became the preferred way for handling user information, leaving :attr:`.Request.meta` - for communication with components like middlewares and extensions. - -.. _topics-request-response-ref-errbacks: - -Using errbacks to catch exceptions in request processing --------------------------------------------------------- - -The errback of a request is a function that will be called when an exception -is raise while processing it. - -It receives a :exc:`~twisted.python.failure.Failure` as first parameter and can -be used to track connection establishment timeouts, DNS errors etc. - -Here's an example spider logging all errors and catching some specific -errors if needed: - -.. code-block:: python - - import scrapy - - from scrapy.spidermiddlewares.httperror import HttpError - from twisted.internet.error import DNSLookupError - from twisted.internet.error import TimeoutError, TCPTimedOutError - - - class ErrbackSpider(scrapy.Spider): - name = "errback_example" - start_urls = [ - "http://www.httpbin.org/", # HTTP 200 expected - "http://www.httpbin.org/status/404", # Not found error - "http://www.httpbin.org/status/500", # server issue - "http://www.httpbin.org:12345/", # non-responding host, timeout expected - "https://example.invalid/", # DNS error expected - ] - - async def start(self): - for u in self.start_urls: - yield scrapy.Request( - u, - callback=self.parse_httpbin, - errback=self.errback_httpbin, - dont_filter=True, - ) - - def parse_httpbin(self, response): - self.logger.info(f"Got successful response from {response.url}") - # do something useful here... - - def errback_httpbin(self, failure): - # log all failures - self.logger.error(repr(failure)) - - # in case you want to do something special for some errors, - # you may need the failure's type: - - if failure.check(HttpError): - # these exceptions come from HttpError spider middleware - # you can get the non-200 response - response = failure.value.response - self.logger.error("HttpError on %s", response.url) - - elif failure.check(DNSLookupError): - # this is the original request - request = failure.request - self.logger.error("DNSLookupError on %s", request.url) - - elif failure.check(TimeoutError, TCPTimedOutError): - request = failure.request - self.logger.error("TimeoutError on %s", request.url) - - -.. _errback-cb_kwargs: - -Accessing additional data in errback functions ----------------------------------------------- - -In case of a failure to process the request, you may be interested in -accessing arguments to the callback functions so you can process further -based on the arguments in the errback. The following example shows how to -achieve this by using ``Failure.request.cb_kwargs``: - -.. code-block:: python - - def parse(self, response): - request = scrapy.Request( - "http://www.example.com/index.html", - callback=self.parse_page2, - errback=self.errback_page2, - cb_kwargs=dict(main_url=response.url), - ) - yield request - - - def parse_page2(self, response, main_url): - pass - - - def errback_page2(self, failure): - yield dict( - main_url=failure.request.cb_kwargs["main_url"], - ) - - .. _request-fingerprints: Request fingerprints @@ -702,6 +549,319 @@ The following built-in Scrapy components have such restrictions: 45-character-long keys must be supported. +.. _callbacks: + +Callbacks +========= + +A callback is a function that Scrapy calls with the :class:`Response` of a +:class:`~scrapy.Request` once that request has been downloaded, so that you can +extract data from that response and generate additional requests to continue +the crawl: + +.. code-block:: python + + from scrapy import Request, Spider + + + class BookSpider(Spider): + name = "books" + + async def start(self): + yield Request("https://books.toscrape.com/", callback=self.parse_home) + + def parse_home(self, response): + for url in response.css("h3 a::attr(href)").getall(): + yield Request(response.urljoin(url), callback=self.parse_book) + + def parse_book(self, response): + yield {"title": response.css("h1::text").get()} + +Requests may also define an :ref:`errback `, which Scrapy calls +instead of the callback when an exception is raised while processing the +request or its response, e.g. a connection error or, by default, a non-2xx +response. + + +.. _callback-assignment: + +Assigning a callback to a request +--------------------------------- + +To assign a callback to a request, use the ``callback`` parameter of +:class:`~scrapy.Request`, which sets the :attr:`.Request.callback` attribute: + +.. code-block:: python + + from scrapy import Request + + + def parse_home(response): ... + + + request = Request("https://books.toscrape.com/", callback=parse_home) + +Requests with no callback, i.e. with :attr:`~scrapy.Request.callback` set to +``None``, are handled by the :meth:`~scrapy.Spider.parse` method of the spider: + +.. code-block:: python + + request = Request("https://books.toscrape.com/") # Handled by parse() + +If a request is never meant to reach a spider callback, e.g. because a +:ref:`component ` sends it and handles its response itself, +assign the special :func:`~scrapy.http.request.NO_CALLBACK` value to it +instead, so that :ref:`downloader middlewares ` +can tell such requests apart. + +While :attr:`~scrapy.Request.callback` only accepts callables, some spider +classes let you also define a callback by name: both :attr:`CrawlSpider.rules +` and :attr:`SitemapSpider.sitemap_rules +` accept the name of a spider +method as a string. + + +.. _writing-callbacks: + +Writing a callback +------------------ + +Any callable can be a callback, as long as it takes the response as its first +positional parameter, and any :ref:`additional callback data ` +as keyword parameters. Spider methods are the most common choice, but plain +functions, lambda expressions and other callable objects work as well. + +.. note:: If you enable :ref:`job persistence ` through the + :setting:`JOBDIR` setting, callbacks must be methods of the running spider. + Requests with any other callback cannot be serialized, so they are kept in + memory only and lost when you pause the crawl. See + :ref:`request-serialization`. + +A callback can be: + +- A regular function: + + .. code-block:: python + + def parse(self, response): + return {"url": response.url} + +- A generator function: + + .. code-block:: python + + def parse(self, response): + yield {"url": response.url} + +- A coroutine function, i.e. defined with ``async def``: + + .. code-block:: python + + async def parse(self, response): + return {"url": response.url} + +- An asynchronous generator function: + + .. code-block:: python + + async def parse(self, response): + yield {"url": response.url} + +The last two allow using ``await``, ``async for`` and ``async with`` in your +callback. See :ref:`topics-coroutines`. + + +.. _callback-output: + +Callback output +--------------- + +A callback may return or yield any of the following: + +- ``None``, which does nothing. + + Callbacks that produce no output at all, e.g. callbacks that only log + information about the response, are perfectly valid. ``None`` values within + an iterable of callback output are ignored as well. + +- A :class:`~scrapy.Request` object, which Scrapy schedules, downloads and + eventually sends to its own callback. + +- An :ref:`item object `, which Scrapy sends to the + :ref:`item pipelines `. + + Any object that is neither ``None`` nor a :class:`~scrapy.Request` object + is treated as an item. + +- An iterable of any of the values above, e.g. a list or, more commonly, a + generator. + + :term:`Asynchronous iterables `, e.g. an + :term:`asynchronous generator`, are also supported. + +.. note:: When a callback *returns* an object, Scrapy iterates that object if + it supports iteration, except for :class:`dict`, :class:`~scrapy.Item`, + :class:`str` and :class:`bytes` objects, which are always handled as single + items. + +.. note:: In a generator callback, a ``return`` statement with a value does not + produce any output, since such a value is not part of what the generator + yields. Scrapy logs a warning when it detects such a callback, see + :setting:`WARN_ON_GENERATOR_RETURN_VALUE`. + +Before Scrapy acts on the output of a callback, that output goes through the +:meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_spider_output` method +of your :ref:`spider middlewares `, which may modify +it or drop part of it. + +If a callback raises an exception, the :attr:`~scrapy.Request.errback` of the +request is *not* called. The exception goes through the +:meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_spider_exception` +method of your spider middlewares instead and, unless one of them handles it, +Scrapy logs it and sends the :signal:`spider_error` signal. + + +.. _callback-data: +.. _topics-request-response-ref-request-callback-arguments: + +Passing additional data to callback functions +--------------------------------------------- + +In some cases you may be interested in passing data to a callback in addition +to the response, e.g. data extracted from the response that triggered the +request. The following example shows how to achieve this by using the +:attr:`.Request.cb_kwargs` attribute: + +.. code-block:: python + + from scrapy import Request + + + def parse(self, response): + request = Request( + "http://www.example.com/index.html", + callback=self.parse_page2, + cb_kwargs=dict(main_url=response.url), + ) + request.cb_kwargs["foo"] = "bar" # add more arguments for the callback + yield request + + + def parse_page2(self, response, main_url, foo): + yield dict( + main_url=main_url, + other_url=response.url, + foo=foo, + ) + +:attr:`.Request.cb_kwargs` is the recommended way to pass your own data to a +callback. Use :attr:`.Request.meta` only for data aimed at :ref:`components +`, such as middlewares and extensions. + +.. _errbacks: +.. _topics-request-response-ref-errbacks: + +Errbacks +======== + +The errback of a request is a function that will be called when an exception +is raise while processing it. + +It receives a :exc:`~twisted.python.failure.Failure` as first parameter and can +be used to track connection establishment timeouts, DNS errors etc. + +Here's an example spider logging all errors and catching some specific +errors if needed: + +.. code-block:: python + + from scrapy import Request, Spider + from scrapy.spidermiddlewares.httperror import HttpError + from twisted.internet.error import DNSLookupError + from twisted.internet.error import TimeoutError, TCPTimedOutError + + + class ErrbackSpider(Spider): + name = "errback_example" + start_urls = [ + "http://www.httpbin.org/", # HTTP 200 expected + "http://www.httpbin.org/status/404", # Not found error + "http://www.httpbin.org/status/500", # server issue + "http://www.httpbin.org:12345/", # non-responding host, timeout expected + "https://example.invalid/", # DNS error expected + ] + + async def start(self): + for u in self.start_urls: + yield Request( + u, + callback=self.parse_httpbin, + errback=self.errback_httpbin, + dont_filter=True, + ) + + def parse_httpbin(self, response): + self.logger.info(f"Got successful response from {response.url}") + # do something useful here... + + def errback_httpbin(self, failure): + # log all failures + self.logger.error(repr(failure)) + + # in case you want to do something special for some errors, + # you may need the failure's type: + + if failure.check(HttpError): + # these exceptions come from HttpError spider middleware + # you can get the non-200 response + response = failure.value.response + self.logger.error("HttpError on %s", response.url) + + elif failure.check(DNSLookupError): + # this is the original request + request = failure.request + self.logger.error("DNSLookupError on %s", request.url) + + elif failure.check(TimeoutError, TCPTimedOutError): + request = failure.request + self.logger.error("TimeoutError on %s", request.url) + + +.. _errback-cb_kwargs: + +Accessing additional data in errback functions +---------------------------------------------- + +In case of a failure to process the request, you may be interested in +accessing arguments to the callback functions so you can process further +based on the arguments in the errback. The following example shows how to +achieve this by using ``Failure.request.cb_kwargs``: + +.. code-block:: python + + from scrapy import Request + + + def parse(self, response): + request = Request( + "http://www.example.com/index.html", + callback=self.parse_page2, + errback=self.errback_page2, + cb_kwargs=dict(main_url=response.url), + ) + yield request + + + def parse_page2(self, response, main_url): + pass + + + def errback_page2(self, failure): + yield dict( + main_url=failure.request.cb_kwargs["main_url"], + ) + + .. _topics-request-meta: Request.meta special keys diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 69ff08fa1..8fbf0c52d 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -4,43 +4,31 @@ Spiders ======= -Spiders are classes which define how a certain site (or a group of sites) will be -scraped, including how to perform the crawl (i.e. follow links) and how to -extract structured data from their pages (i.e. scraping items). In other words, -Spiders are the place where you define the custom behaviour for crawling and -parsing pages for a particular site (or, in some cases, a group of sites). +Spiders are classes that define how a site, or a group of sites, is scraped: +which requests to send, and how to parse their responses to extract data and to +send additional requests. -For spiders, the scraping cycle goes through something like this: +A crawl goes as follows: -1. You start by generating the initial requests to crawl the first URLs, and - specify a callback function to be called with the response downloaded from - those requests. +1. Scrapy iterates the :meth:`~scrapy.Spider.start` method of the spider to + get the initial requests. By default, that method yields a + :class:`~scrapy.Request` object for each URL in + :attr:`~scrapy.Spider.start_urls`, with :meth:`~scrapy.Spider.parse` as + :ref:`callback `. - The first requests to perform are obtained by iterating the - :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` - function to handle each :class:`~scrapy.http.Response`. +2. Scrapy downloads each request and calls its callback with the resulting + :class:`~scrapy.http.Response`. -2. In the callback function, you parse the response (web page) and return - :ref:`item objects `, - :class:`~scrapy.Request` objects, or an iterable of these objects. - Those Requests will also contain a callback (maybe - the same) and will then be downloaded by Scrapy and then their - response handled by the specified callback. +3. Callbacks parse the response, typically using :ref:`topics-selectors`, and + return or yield :ref:`item objects ` with the extracted data + and :class:`~scrapy.Request` objects to continue the crawl, which go back + to step 2. See :ref:`callback-output`. -3. In callback functions, you parse the page contents, typically using - :ref:`topics-selectors` (but you can also use BeautifulSoup, lxml or whatever - mechanism you prefer) and generate items with the parsed data. +4. Items go through :ref:`item pipelines `, and are + usually stored through :ref:`topics-feed-exports`. -4. Finally, the items returned from the spider will be typically persisted to a - database (in some :ref:`Item Pipeline `) or written to - a file using :ref:`topics-feed-exports`. - -Even though this cycle applies (more or less) to any kind of spider, there are -different kinds of default spiders bundled into Scrapy for different purposes. -We will talk about those types here. +Scrapy includes different spider classes for different purposes, described +below. .. _topics-spiders-ref: @@ -191,22 +179,7 @@ scrapy.Spider .. automethod:: start - .. method:: parse(response) - - This is the default callback used by Scrapy to process downloaded - responses, when their requests don't specify a callback. - - The ``parse`` method is in charge of processing the response and returning - scraped data and/or more URLs to follow. Other Requests callbacks have - the same requirements as the :class:`~scrapy.Spider` class. - - This method, as well as any other Request callback, must return a - :class:`~scrapy.Request` object, an :ref:`item object `, an - iterable of :class:`~scrapy.Request` objects and/or :ref:`item objects - `, or ``None``. - - :param response: the response to parse - :type response: :class:`~scrapy.http.Response` + .. automethod:: parse .. method:: closed(reason) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 68847283a..7d67bb6d7 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -169,7 +169,8 @@ class Request(object_ref): #: #: The callable must expect the response as its first parameter, and #: support any additional keyword arguments set through - #: :attr:`cb_kwargs`. + #: :attr:`cb_kwargs`. See :ref:`writing-callbacks` and + #: :ref:`callback-output`. #: #: In addition to an arbitrary callable, the following values are also #: supported: @@ -190,8 +191,7 @@ class Request(object_ref): #: raises exceptions for non-2xx responses by default, sending them #: to the :attr:`errback` instead. #: - #: .. seealso:: - #: :ref:`topics-request-response-ref-request-callback-arguments` + #: .. seealso:: :ref:`callbacks` self.callback: CallbackT | None = callback #: :class:`~collections.abc.Callable` to handle exceptions raised @@ -200,7 +200,7 @@ class Request(object_ref): #: The callable must expect a :exc:`~twisted.python.failure.Failure` as #: its first parameter. #: - #: .. seealso:: :ref:`topics-request-response-ref-errbacks` + #: .. seealso:: :ref:`errbacks` self.errback: Callable[[Failure], Any] | None = errback self._cookies: CookiesT | None = cookies or None diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 02dfa2ac6..6244e3264 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -143,6 +143,22 @@ class Spider(object_ref): else: def parse(self, response: Response, **kwargs: Any) -> Any: + """Process *response*, i.e. extract data from it and generate new + requests. + + This is the default :ref:`callback `: Scrapy uses + it for the response to any request that does not define a + :attr:`~scrapy.Request.callback`, such as the requests that + :meth:`start` yields by default. + + Any :attr:`~scrapy.Request.cb_kwargs` of the request are passed as + keyword parameters. + + Spiders must define this method, unless every request that they + send defines a callback. + + See :ref:`callback-output` about the supported return values. + """ raise NotImplementedError( f"{self.__class__.__name__}.parse callback is not defined" ) From 3fc7148c5ec7537c000af27b705e33d797429a3a Mon Sep 17 00:00:00 2001 From: Janit Rajkarnikar <108281535+aniJani@users.noreply.github.com> Date: Thu, 30 Jul 2026 10:18:45 -0500 Subject: [PATCH 6/7] Don't evaluate annotations when inspecting signatures (#7818) Since Python 3.14 (PEP 649) annotations are evaluated lazily, so inspect.signature() raises NameError for callables whose annotations reference names imported only under TYPE_CHECKING. This broke middleware registration (via argument_is_required()) and custom stats collectors (via _warn_spider_arg) for user code with such annotations. Use annotation_format=Format.FORWARDREF on 3.14+: parameter names, kinds and defaults are unchanged, and unresolvable annotations become ForwardRef proxies instead of raising. Resolves #7796. --- scrapy/utils/decorators.py | 3 ++- scrapy/utils/python.py | 21 ++++++++++++++++++++- tests/test_utils_decorators.py | 30 ++++++++++++++++++++++++++++-- tests/test_utils_python.py | 21 ++++++++++++++++++++- 4 files changed, 70 insertions(+), 5 deletions(-) diff --git a/scrapy/utils/decorators.py b/scrapy/utils/decorators.py index 4960dc27a..2924c81f9 100644 --- a/scrapy/utils/decorators.py +++ b/scrapy/utils/decorators.py @@ -10,6 +10,7 @@ from twisted.internet.defer import Deferred, maybeDeferred from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.asyncio import run_in_thread from scrapy.utils.defer import deferred_from_coro +from scrapy.utils.python import _signature if TYPE_CHECKING: from collections.abc import AsyncGenerator, Callable, Coroutine @@ -109,7 +110,7 @@ def _warn_spider_arg( ): """Decorator to warn if a ``spider`` argument is passed to a function.""" - sig = inspect.signature(func) + sig = _signature(func) def check_args(*args: _P.args, **kwargs: _P.kwargs) -> None: bound = sig.bind(*args, **kwargs) diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 8a7517c1d..40fc05257 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -178,11 +178,30 @@ def binary_is_text(data: bytes) -> bool: return all(c not in _BINARYCHARS for c in data) +# PEP 649 (Python 3.14+) made annotation evaluation lazy, so inspect.signature() +# can raise NameError for names imported only under TYPE_CHECKING. We only need +# parameter names, kinds and defaults, so leave such annotations as ForwardRefs. +if sys.version_info >= (3, 14): + from annotationlib import Format + + def _signature(func: Callable[..., Any]) -> inspect.Signature: + return inspect.signature(func, annotation_format=Format.FORWARDREF) + +else: + + def _signature(func: Callable[..., Any]) -> inspect.Signature: + return inspect.signature(func) + + def get_func_args_dict( func: Callable[..., Any], stripself: bool = False ) -> Mapping[str, inspect.Parameter]: """Return the argument dict of a callable object. + Annotations are not evaluated, so on Python 3.14 and later the ``annotation`` + attribute of the returned parameters may be a ``ForwardRef`` instead of the + resolved type. + .. versionadded:: 2.14 """ if not callable(func): @@ -190,7 +209,7 @@ def get_func_args_dict( args: Mapping[str, inspect.Parameter] try: - sig = inspect.signature(func) + sig = _signature(func) except ValueError: return {} diff --git a/tests/test_utils_decorators.py b/tests/test_utils_decorators.py index 807294a57..4c29d2917 100644 --- a/tests/test_utils_decorators.py +++ b/tests/test_utils_decorators.py @@ -1,7 +1,8 @@ from __future__ import annotations +import sys import warnings -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any import pytest from twisted.internet.defer import Deferred @@ -12,7 +13,7 @@ from scrapy.utils.defer import maybe_deferred_to_future from tests.utils.decorators import coroutine_test if TYPE_CHECKING: - from collections.abc import AsyncGenerator + from collections.abc import AsyncGenerator, Callable class TestDeprecated: @@ -77,6 +78,31 @@ class TestWarnSpiderArg: ): assert parse("response", spider="spider") == "response" + @pytest.mark.skipif( + sys.version_info < (3, 14), + reason="annotations are only lazily evaluated since Python 3.14 (PEP 649)", + ) + def test_sync_warns_with_unresolvable_annotations(self): + # dont_inherit=True, or the module's future import stringizes the annotations + namespace: dict[str, Any] = {} + exec( # pylint: disable=exec-used + compile( + "def parse(response: OnlyAtTypeCheckingTime," + " spider: OnlyAtTypeCheckingTime | None = None): return response", + "", + "exec", + dont_inherit=True, + ), + namespace, + ) + parse_func: Callable[..., str] = namespace["parse"] + parse = _warn_spider_arg(parse_func) + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Passing a 'spider' argument" + ): + assert parse("response", spider="spider") == "response" + def test_sync_no_warning_without_spider_arg(self): @_warn_spider_arg def parse(response: str, spider: str | None = None) -> str: diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index c3b5dfc99..099b5ccc2 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -4,7 +4,7 @@ import functools import operator import platform import sys -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING, Any, TypeVar import pytest @@ -190,6 +190,25 @@ def test_get_func_args(): ] +@pytest.mark.skipif( + sys.version_info < (3, 14), + reason="annotations are only lazily evaluated since Python 3.14 (PEP 649)", +) +def test_get_func_args_unresolvable_annotations(): + # dont_inherit=True, or the module's future import stringizes the annotations + namespace: dict[str, Any] = {} + exec( # pylint: disable=exec-used + compile( + "def f(a: OnlyAtTypeCheckingTime, b: int = 1) -> OnlyAtTypeCheckingTime: pass", + "", + "exec", + dont_inherit=True, + ), + namespace, + ) + assert get_func_args(namespace["f"]) == ["a", "b"] + + @pytest.mark.parametrize( ("value", "expected"), [ From 7436afc95f521482b1b1473c88f6b5ab1d430bea Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 17:27:27 +0200 Subject: [PATCH 7/7] Improve test coverage for scrapy.extensions (#7809) --- scrapy/extensions/closespider.py | 2 +- scrapy/extensions/feedexport.py | 4 +- scrapy/extensions/memusage.py | 2 +- scrapy/extensions/periodic_log.py | 3 +- tests/test_downloadermiddleware_httpcache.py | 71 ++++++++++++++++++++ tests/test_extension_memusage.py | 66 +++++++++++++++++- tests/test_extension_periodic_log.py | 36 ++++++++++ tests/test_extension_telnet.py | 55 ++++++++++++++- tests/test_feedexport.py | 26 +++++++ tests/test_feedexport_postprocess.py | 10 +++ tests/test_feedexport_storages.py | 15 +++++ 11 files changed, 278 insertions(+), 12 deletions(-) diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py index a4362b182..9cb792e30 100644 --- a/scrapy/extensions/closespider.py +++ b/scrapy/extensions/closespider.py @@ -119,7 +119,7 @@ class CloseSpider: self.task = None if self.task_no_item: - if self.task_no_item.running: + if self.task_no_item.running: # pragma: no branch self.task_no_item.stop() self.task_no_item = None diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 448279546..118462bc9 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -465,7 +465,7 @@ class FeedSlot: ) def finish_exporting(self) -> None: - if self._exporting: + if self._exporting: # pragma: no branch assert self.exporter self.exporter.finish_exporting() self._exporting = False @@ -553,7 +553,7 @@ class FeedExporter: for slot in self.slots: self._schedule_slot_close(slot, spider) - if self._pending_close_tasks: + if self._pending_close_tasks: # pragma: no branch if is_asyncio_available(): await asyncio.wait( cast("list[asyncio.Task[None]]", list(self._pending_close_tasks)) diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index 1444c8941..e0e289ce8 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -94,7 +94,7 @@ class MemoryUsage: def engine_stopped(self) -> None: for tsk in self.tasks: - if tsk.running: + if tsk.running: # pragma: no branch tsk.stop() def update(self) -> None: diff --git a/scrapy/extensions/periodic_log.py b/scrapy/extensions/periodic_log.py index cd35c8165..cbcc8b70e 100644 --- a/scrapy/extensions/periodic_log.py +++ b/scrapy/extensions/periodic_log.py @@ -38,7 +38,6 @@ class PeriodicLog: ): self.stats: StatsCollector = stats self.interval: float = interval - self.multiplier: float = 60.0 / self.interval self.task: AsyncioLoopingCall | LoopingCall | None = None self.encoder: JSONEncoder = ScrapyJSONEncoder(sort_keys=True, indent=4) self.ext_stats_enabled: bool = bool(ext_stats) @@ -165,5 +164,5 @@ class PeriodicLog: def spider_closed(self, spider: Spider, reason: str) -> None: self.log() - if self.task and self.task.running: + if self.task and self.task.running: # pragma: no branch self.task.stop() diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index ce56ee11d..6e8486eb8 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -14,6 +14,7 @@ import pytest from scrapy.downloadermiddlewares.httpcache import HttpCacheMiddleware from scrapy.exceptions import IgnoreRequest +from scrapy.extensions.httpcache import DummyPolicy from scrapy.http import HtmlResponse, Request, Response from scrapy.spiders import Spider from scrapy.utils.test import get_crawler @@ -24,6 +25,14 @@ if TYPE_CHECKING: from scrapy.crawler import Crawler +class AlwaysStalePolicy(DummyPolicy): + """:class:`~scrapy.extensions.httpcache.DummyPolicy` that always + revalidates cached responses.""" + + def is_cached_response_fresh(self, cachedresponse, request): + return False + + class TestBase: """Base class with common setup and helper methods.""" @@ -282,6 +291,21 @@ class DummyPolicyTestMixin(PolicyTestMixin): self.assertEqualResponse(self.response, response) assert "cached" in response.flags + def test_revalidation_keeps_cached_response(self): + # The dummy policy considers every cached response valid, so a policy + # that subclasses it to force revalidation always gets the cached + # response back, whatever the new response is. + with self._middleware(HTTPCACHE_POLICY=AlwaysStalePolicy) as mw: + assert mw.process_request(self.request) is None + mw.process_response(self.request, self.response) + + assert mw.process_request(self.request) is None + fresh_response = self.response.replace(body=b"new body") + response = mw.process_response(self.request, fresh_response) + self.assertEqualResponse(self.response, response) + assert "cached" in response.flags + assert mw.stats.get_value("httpcache/revalidate") == 1 + class RFC2616PolicyTestMixin(PolicyTestMixin): """Mixin containing RFC2616 policy specific test methods.""" @@ -553,6 +577,53 @@ class RFC2616PolicyTestMixin(PolicyTestMixin): else: assert "cached" in res5.flags + def test_middleware_ignore_schemes(self): + # file responses are not cached by default + req = Request("file:///tmp/t.txt") + res = Response(req.url, headers={"Expires": self.tomorrow}) + with self._middleware() as mw: + assert mw.process_request(req) is None + mw.process_response(req, res) + + assert mw.storage.retrieve_response(mw.crawler.spider, req) is None + assert mw.process_request(req) is None + + def test_max_stale_with_value(self): + # A response that expired one day ago. + headers = {"Date": self.yesterday, "Expires": self.yesterday} + with self._middleware() as mw: + req0 = Request("http://example.com") + res0 = Response(req0.url, headers=headers) + self._process_requestresponse(mw, req0, res0) + + # max-stale greater than the staleness of the cached response + req1 = req0.replace(headers={"Cache-Control": "max-stale=172800"}) + res1 = mw.process_request(req1) + assert isinstance(res1, Response) + assert "cached" in res1.flags + + # max-stale lower than the staleness of the cached response + req2 = req0.replace(headers={"Cache-Control": "max-stale=60"}) + assert mw.process_request(req2) is None + + # a non-integer max-stale value is ignored + req3 = req0.replace(headers={"Cache-Control": "max-stale=soon"}) + assert mw.process_request(req3) is None + + def test_response_dated_in_the_future(self): + # A Date header ahead of the local clock must not make the cached + # response look aged. + headers = {"Date": self.tomorrow, "Cache-Control": "max-age=10"} + with self._middleware() as mw: + req0 = Request("http://example.com") + res0 = Response(req0.url, headers=headers) + res1 = self._process_requestresponse(mw, req0, res0) + assert "cached" not in res1.flags + + res2 = self._process_requestresponse(mw, req0, None) + self.assertEqualResponse(res1, res2) + assert "cached" in res2.flags + def test_process_exception(self): with self._middleware() as mw: res0 = Response(self.request.url, headers={"Expires": self.yesterday}) diff --git a/tests/test_extension_memusage.py b/tests/test_extension_memusage.py index a474725d8..76e8ca5d6 100644 --- a/tests/test_extension_memusage.py +++ b/tests/test_extension_memusage.py @@ -2,6 +2,7 @@ from __future__ import annotations import logging import sys +from typing import TYPE_CHECKING import pytest @@ -13,8 +14,12 @@ from scrapy.extensions.memusage import MemoryUsage from scrapy.spiders import Spider from scrapy.utils.test import get_crawler from tests.utils import OneShotLoop +from tests.utils.cmdline import proc from tests.utils.decorators import coroutine_test +if TYPE_CHECKING: + from tests.mockserver.http import MockServer + # MemoryUsage relies on the stdlib 'resource' module (not available on Windows) pytestmark = pytest.mark.skipif( sys.platform.startswith("win"), @@ -25,6 +30,14 @@ pytestmark = pytest.mark.skipif( MB = 1024 * 1024 +class TwoShotLoop(OneShotLoop): + """Like :class:`OneShotLoop`, but runs the check twice.""" + + def start(self, interval: float, now: bool = True) -> None: + super().start(interval, now=now) + self.func() + + class _LoopSpider(Spider): name = "loop-data-spider" @@ -50,6 +63,49 @@ def test_memusage_disabled() -> None: MemoryUsage.from_crawler(get_crawler(settings_dict=settings)) +def test_memusage_limit_stops_crawler_without_spider(mockserver: MockServer) -> None: + # The Scrapy shell starts the engine without opening a spider, so the + # whole crawler is stopped instead of a spider being closed. + _, out, err = proc( + "shell", + mockserver.url("/text"), + "-c", + "response.status", + "--set", + "MEMUSAGE_LIMIT_MB=1", + ) + assert "Memory usage exceeded 1MiB" in err + assert "200" in out + + +@coroutine_test +async def test_memusage_below_thresholds_logs_peak( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + settings = { + "MEMUSAGE_LIMIT_MB": 100, + "MEMUSAGE_WARNING_MB": 50, + "MEMUSAGE_CHECK_INTERVAL_SECONDS": 0.01, + "TELNETCONSOLE_ENABLED": False, + "LOG_LEVEL": "INFO", + } + + monkeypatch.setattr(memusage_mod, "create_looping_call", OneShotLoop) + monkeypatch.setattr(MemoryUsage, "get_virtual_size", lambda _: 25 * MB) + + crawler = get_crawler(spidercls=_LoopSpider, settings_dict=settings) + + with caplog.at_level(logging.INFO, logger="scrapy.extensions.memusage"): + await crawler.crawl_async(url="data:,", loops=1) + + assert crawler.stats + assert crawler.stats.get_value("memusage/limit_reached") is None + assert crawler.stats.get_value("memusage/warning_reached") is None + assert crawler.stats.get_value("memusage/max") == 25 * MB + assert crawler.stats.get_value("finish_reason") == "finished" + assert any("Peak memory usage is 25MiB" in r.getMessage() for r in caplog.records) + + @coroutine_test async def test_memusage_limit_closes_spider_with_reason_and_error_log( caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch @@ -92,8 +148,9 @@ async def test_memusage_warning_logs_but_allows_normal_finish( "LOG_LEVEL": "INFO", } - # Avoid background LoopingCall that can log after the test finishes. - monkeypatch.setattr(memusage_mod, "create_looping_call", OneShotLoop) + # Avoid background LoopingCall that can log after the test finishes; check + # twice, since the warning is only meant to be reported once. + monkeypatch.setattr(memusage_mod, "create_looping_call", TwoShotLoop) monkeypatch.setattr(MemoryUsage, "get_virtual_size", lambda self: 75 * MB) crawler = get_crawler(spidercls=_LoopSpider, settings_dict=settings) @@ -112,4 +169,7 @@ async def test_memusage_warning_logs_but_allows_normal_finish( assert crawler.stats assert crawler.stats.get_value("memusage/warning_reached") == 1 assert crawler.stats.get_value("finish_reason") == "finished" - assert any("memory usage reached" in r.getMessage().lower() for r in caplog.records) + warnings_logged = [ + r for r in caplog.records if "memory usage reached" in r.getMessage().lower() + ] + assert len(warnings_logged) == 1 diff --git a/tests/test_extension_periodic_log.py b/tests/test_extension_periodic_log.py index ffe7a0dc7..723fb6e17 100644 --- a/tests/test_extension_periodic_log.py +++ b/tests/test_extension_periodic_log.py @@ -1,8 +1,13 @@ from __future__ import annotations import datetime +import json +import logging from typing import TYPE_CHECKING, Any +import pytest + +from scrapy.exceptions import NotConfigured from scrapy.extensions.periodic_log import PeriodicLog from scrapy.utils.test import get_crawler @@ -86,6 +91,14 @@ class TestPeriodicLog: assert extension({"PERIODIC_LOG_DELTA": True, "LOGSTATS_INTERVAL": 60}) assert extension({"PERIODIC_LOG_DELTA": "True", "LOGSTATS_INTERVAL": 60}) + def test_no_interval(self): + with pytest.raises(NotConfigured): + extension({"PERIODIC_LOG_STATS": True, "LOGSTATS_INTERVAL": 0}) + + def test_nothing_enabled(self): + with pytest.raises(NotConfigured): + extension({"LOGSTATS_INTERVAL": 60}) + @coroutine_test async def test_log_delta(self): def emulate( @@ -212,3 +225,26 @@ class TestPeriodicLog: {"PERIODIC_LOG_STATS": {"include": ["downloader/"], "exclude": ["bytes"]}}, lambda k, v: "downloader/" in k and "bytes" not in k, ) + + @coroutine_test + async def test_log_timing(self, caplog: pytest.LogCaptureFixture) -> None: + settings = { + "EXTENSIONS": {"scrapy.extensions.periodic_log.PeriodicLog": 0}, + "PERIODIC_LOG_TIMING_ENABLED": True, + "LOGSTATS_INTERVAL": 30, + } + crawler = get_crawler(MetaSpider, settings) + with caplog.at_level(logging.INFO, logger="scrapy.extensions.periodic_log"): + await crawler.crawl_async() + + records = [ + r for r in caplog.records if r.name == "scrapy.extensions.periodic_log" + ] + assert records, "PeriodicLog logged nothing" + # Only the timing section is enabled, and it is logged on spider close. + data = json.loads(records[-1].getMessage()) + assert list(data) == ["time"] + assert data["time"]["log_interval"] == 30 + assert data["time"]["log_interval_real"] >= 0 + assert data["time"]["elapsed"] >= 0 + assert data["time"]["start_time"] <= data["time"]["utcnow"] diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index 20c801558..fca0e3153 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -7,7 +7,8 @@ import pytest from twisted.conch.telnet import ITelnetProtocol from twisted.cred import credentials -from scrapy.extensions.telnet import TelnetConsole +from scrapy import Spider +from scrapy.extensions.telnet import TelnetConsole, update_telnet_vars from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.test import get_crawler from tests.utils.decorators import coroutine_test @@ -16,16 +17,20 @@ if TYPE_CHECKING: from collections.abc import Generator from scrapy.crawler import Crawler + from scrapy.http import Response pytestmark = pytest.mark.requires_reactor # TelnetConsole requires a reactor -def _get_crawler(settings_dict: dict[str, Any] | None = None) -> Crawler: +def _get_crawler( + spidercls: type[Spider] | None = None, + settings_dict: dict[str, Any] | None = None, +) -> Crawler: settings = { "TELNETCONSOLE_ENABLED": True, **(settings_dict or {}), } - return get_crawler(settings_dict=settings) + return get_crawler(spidercls, settings_dict=settings) @contextmanager @@ -84,3 +89,47 @@ def test_invalid_reversed_portrange() -> None: console = TelnetConsole(_get_crawler(settings_dict=settings)) with pytest.raises(ValueError, match=r"invalid portrange: \[2, 1\]"): console.start_listening() + + +@coroutine_test +async def test_telnet_vars() -> None: + """Log into the console of a running crawl, which is when the telnet + variables are built.""" + received: list[dict[str, Any]] = [] + + def on_update_telnet_vars(telnet_vars: dict[str, Any]) -> None: + received.append(telnet_vars) + + class TelnetSpider(Spider): + name = "telnet" + start_urls = ["data:,"] + + async def parse(self, response: Response) -> None: + assert self.crawler.extensions + console = next( + ext + for ext in self.crawler.extensions.middlewares + if isinstance(ext, TelnetConsole) + ) + creds = credentials.UsernamePassword( + console.username.encode("utf8"), console.password.encode("utf8") + ) + portal = console.protocol().protocolArgs[0] + await maybe_deferred_to_future(portal.login(creds, None, ITelnetProtocol)) + + crawler = _get_crawler(TelnetSpider) + crawler.signals.connect(on_update_telnet_vars, signal=update_telnet_vars) + await crawler.crawl_async() + + assert len(received) == 1 + telnet_vars = received[0] + assert telnet_vars["crawler"] is crawler + assert telnet_vars["engine"] is crawler.engine + assert telnet_vars["spider"] is crawler.spider + assert telnet_vars["extensions"] is crawler.extensions + assert telnet_vars["stats"] is crawler.stats + assert telnet_vars["settings"] is crawler.settings + assert callable(telnet_vars["est"]) + assert callable(telnet_vars["p"]) + assert callable(telnet_vars["prefs"]) + assert "telnetconsole.html" in telnet_vars["help"] diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 1cca287af..40a763efd 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -24,6 +24,7 @@ from scrapy.extensions.feedexport import ( FeedExporter, FeedSlot, FileFeedStorage, + ItemFilter, apply_uri_params, ) from scrapy.utils.python import to_unicode @@ -1289,6 +1290,13 @@ class TestFeedExporterSignals: assert self.feed_exporter_closed_received +class TestItemFilter: + def test_no_feed_options(self): + item_filter = ItemFilter(None) + assert item_filter.item_classes == () + assert item_filter.accepts(MyItem({"foo": "bar"})) + + class TestFeedExportInit: def test_unsupported_storage(self): settings = { @@ -1300,6 +1308,24 @@ class TestFeedExportInit: with pytest.raises(NotConfigured): FeedExporter.from_crawler(crawler) + def test_disabled_storage(self, caplog: pytest.LogCaptureFixture): + class DisabledFeedStorage: + def __init__(self, uri, *, feed_options=None): + raise NotConfigured("not today") + + settings = { + "FEED_STORAGES": {"disabled": DisabledFeedStorage}, + "FEEDS": { + "disabled://uri": {}, + }, + } + crawler = get_crawler(settings_dict=settings) + with caplog.at_level(logging.ERROR), pytest.raises(NotConfigured): + FeedExporter.from_crawler(crawler) + assert ( + "Disabled feed storage scheme: disabled. Reason: not today" in caplog.text + ) + def test_unsupported_format(self): settings = { "FEEDS": { diff --git a/tests/test_feedexport_postprocess.py b/tests/test_feedexport_postprocess.py index f120ce36f..36d8586ce 100644 --- a/tests/test_feedexport_postprocess.py +++ b/tests/test_feedexport_postprocess.py @@ -12,6 +12,7 @@ from typing import TYPE_CHECKING, Any import pytest +from scrapy.extensions.postprocessing import PostProcessingManager from scrapy.utils.test import get_crawler from tests.utils.bases.feedexport import TestFeedExportBase from tests.utils.decorators import coroutine_test @@ -87,6 +88,15 @@ class TestFeedPostProcessedExports(TestFeedExportBase): data_stream.seek(0) return data_stream.read() + def test_tell_reports_target_file_position(self): + """Exporters that wrap the file they get, e.g. through + :class:`io.TextIOWrapper`, need it to report a position.""" + file = BytesIO() + manager = PostProcessingManager([self.MyPlugin1], file, {}) + assert manager.tell() == 0 + manager.write(b"foo") + assert manager.tell() == file.tell() == 3 + @coroutine_test async def test_gzip_plugin(self): filename = self._named_tempfile("gzip_file") diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index b1e3787fd..4d28872b7 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging import os import string +import sys import tempfile from io import BytesIO from pathlib import Path @@ -14,6 +15,7 @@ import pytest from w3lib.url import path_to_file_uri import scrapy +from scrapy.exceptions import NotConfigured from scrapy.extensions.feedexport import ( BlockingFeedStorage, FileFeedStorage, @@ -166,6 +168,12 @@ class TestFTPFeedStorage: st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path", {}) assert st.password == string.punctuation + def test_uri_without_hostname(self): + with pytest.raises( + ValueError, match="Got a storage URI without a hostname: ftp:///some_path" + ): + FTPFeedStorage("ftp:///some_path") + class MyBlockingFeedStorage(BlockingFeedStorage): def _store_in_thread(self, file: IO[bytes]) -> None: @@ -205,6 +213,13 @@ class TestBlockingFeedStorage: b.open(spider=spider) +def test_s3_without_boto3(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setitem(sys.modules, "boto3", None) + monkeypatch.setitem(sys.modules, "boto3.session", None) + with pytest.raises(NotConfigured, match="missing boto3 library"): + S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") + + @pytest.mark.requires_boto3 class TestS3FeedStorage: def test_parse_credentials(self):