From 50500a6b2897418405b459b6a2a80a7a5b0a6f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 19 Jan 2023 17:14:18 +0100 Subject: [PATCH 01/11] Implement a NO_CALLBACK value for Request.callback --- docs/topics/request-response.rst | 43 ++++++++++++++------ scrapy/downloadermiddlewares/robotstxt.py | 4 +- scrapy/http/request/__init__.py | 28 ++++++++++--- scrapy/pipelines/media.py | 3 +- tests/test_downloadermiddleware_robotstxt.py | 8 ++++ tests/test_http_request.py | 9 ++++ tests/test_pipeline_files.py | 5 +-- tests/test_pipeline_images.py | 7 +--- tests/test_pipeline_media.py | 2 + 9 files changed, 79 insertions(+), 30 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index a0d9fc03e..766710d66 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -32,11 +32,22 @@ Request objects :type url: str :param callback: the function that will be called with the response of this - request (once it's downloaded) as its first parameter. For more information - see :ref:`topics-request-response-ref-request-callback-arguments` below. - If a Request doesn't specify a callback, the spider's - :meth:`~scrapy.Spider.parse` method will be used. - Note that if exceptions are raised during processing, errback is called instead. + request (once it's downloaded) as its first parameter. + + In addition to a function, the following values are supported: + + - ``None`` (default), which indicates that the spider's + :meth:`~scrapy.Spider.parse` method must be used. + + - :py:data:`scrapy.http.request.NO_CALLBACK` + + .. autodata:: scrapy.http.request.NO_CALLBACK + + For more information, see + :ref:`topics-request-response-ref-request-callback-arguments`. + + .. note:: If exceptions are raised during processing, ``errback`` is + called instead. :type callback: collections.abc.Callable @@ -69,16 +80,24 @@ Request objects 1. Using a dict:: - request_with_cookies = Request(url="http://www.example.com", - cookies={'currency': 'USD', 'country': 'UY'}) + request_with_cookies = Request( + url="http://www.example.com", + cookies={'currency': 'USD', 'country': 'UY'}, + ) 2. Using a list of dicts:: - request_with_cookies = Request(url="http://www.example.com", - cookies=[{'name': 'currency', - 'value': 'USD', - 'domain': 'example.com', - 'path': '/currency'}]) + request_with_cookies = Request( + url="http://www.example.com", + cookies=[ + { + 'name': 'currency', + 'value': 'USD', + 'domain': 'example.com', + 'path': '/currency', + }, + ], + ) The latter form allows for customizing the ``domain`` and ``path`` attributes of the cookie. This is only useful if the cookies are saved diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index 7bd39aa43..67e14b7b5 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -9,6 +9,7 @@ import logging from twisted.internet.defer import Deferred, maybeDeferred from scrapy.exceptions import NotConfigured, IgnoreRequest from scrapy.http import Request +from scrapy.http.request import NO_CALLBACK from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.log import failure_to_exc_info from scrapy.utils.misc import load_object @@ -65,7 +66,8 @@ class RobotsTxtMiddleware: robotsreq = Request( robotsurl, priority=self.DOWNLOAD_PRIORITY, - meta={'dont_obey_robotstxt': True} + meta={'dont_obey_robotstxt': True}, + callback=NO_CALLBACK, ) dfd = self.crawler.engine.download(robotsreq) dfd.addCallback(self._parse_robots, netloc, spider) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index a1001fc4a..b57faf121 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -20,6 +20,17 @@ from scrapy.utils.url import escape_ajax RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") +#: When assigned to the ``callback`` parameter of +#: :class:`~scrapy.http.Request`, it indicates that the request it not meant to +#: have a spider callback at all. +#: +#: This value should be used by :ref:`components ` +#: that create and handle their own requests, e.g. through +#: :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download +#: middlewares handling such requests can treat them differently from requests +#: intended for the :meth:`~scrapy.Spider.parse` callback. +NO_CALLBACK = object() + class Request(object_ref): """Represents an HTTP request, which is usually generated in a Spider and @@ -63,12 +74,8 @@ class Request(object_ref): raise TypeError(f"Request priority not an integer: {priority!r}") self.priority = priority - if callback is not None and not callable(callback): - raise TypeError(f'callback must be a callable, got {type(callback).__name__}') - if errback is not None and not callable(errback): - raise TypeError(f'errback must be a callable, got {type(errback).__name__}') - self.callback = callback - self.errback = errback + self._set_xback("callback", callback) + self._set_xback("errback", errback) self.cookies = cookies or {} self.headers = Headers(headers or {}, encoding=encoding) @@ -78,6 +85,15 @@ class Request(object_ref): self._cb_kwargs = dict(cb_kwargs) if cb_kwargs else None self.flags = [] if flags is None else list(flags) + def _set_xback(self, name: str, value: Optional[Callable]) -> None: + if ( + value is not None + and (name != "callback" or value is not NO_CALLBACK) + and not callable(value) + ): + raise TypeError(f'{name} must be a callable, got {type(value).__name__}') + setattr(self, name, value) + @property def cb_kwargs(self) -> dict: if self._cb_kwargs is None: diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 5308a9793..fc5db58e8 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -7,6 +7,7 @@ from warnings import warn from twisted.internet.defer import Deferred, DeferredList from twisted.python.failure import Failure +from scrapy.http.request import NO_CALLBACK from scrapy.settings import Settings from scrapy.utils.datatypes import SequenceExclude from scrapy.utils.defer import mustbe_deferred, defer_result @@ -93,7 +94,7 @@ class MediaPipeline: fp = self._fingerprinter.fingerprint(request) cb = request.callback or (lambda _: _) eb = request.errback - request.callback = None + request.callback = NO_CALLBACK request.errback = None # Return cached result if request was already seen diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py index 1460d88eb..71d53ff1a 100644 --- a/tests/test_downloadermiddleware_robotstxt.py +++ b/tests/test_downloadermiddleware_robotstxt.py @@ -8,6 +8,7 @@ from scrapy.downloadermiddlewares.robotstxt import (RobotsTxtMiddleware, logger as mw_module_logger) from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response, TextResponse +from scrapy.http.request import NO_CALLBACK from scrapy.settings import Settings from tests.test_robotstxt_interface import rerp_available, reppy_available @@ -53,6 +54,7 @@ Disallow: /some/randome/page.html middleware = RobotsTxtMiddleware(self._get_successful_crawler()) return DeferredList([ self.assertNotIgnored(Request('http://site.local/allowed'), middleware), + maybeDeferred(self.assertRobotsTxtRequested, "http://site.local"), self.assertIgnored(Request('http://site.local/admin/main'), middleware), self.assertIgnored(Request('http://site.local/static/'), middleware), self.assertIgnored(Request('http://site.local/wiki/K%C3%A4ytt%C3%A4j%C3%A4:'), middleware), @@ -183,6 +185,12 @@ Disallow: /some/randome/page.html return self.assertFailure(maybeDeferred(middleware.process_request, request, spider), IgnoreRequest) + def assertRobotsTxtRequested(self, base_url): + calls = self.crawler.engine.download.call_args_list + request = calls[0][0][0] + self.assertEqual(request.url, f"{base_url}/robots.txt") + self.assertEqual(request.callback, NO_CALLBACK) + class RobotsTxtMiddlewareWithRerpTest(RobotsTxtMiddlewareTest): if not rerp_available(): diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 81cebdc7b..e14f8c8f4 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -7,6 +7,7 @@ from unittest import mock from urllib.parse import parse_qs, unquote_to_bytes, urlparse from scrapy.http import Request, FormRequest, XmlRpcRequest, JsonRequest, Headers, HtmlResponse +from scrapy.http.request import NO_CALLBACK from scrapy.utils.python import to_bytes, to_unicode @@ -277,6 +278,12 @@ class RequestTest(unittest.TestCase): self.assertIs(r4.callback, a_function) self.assertIs(r4.errback, a_function) + r5 = self.request_class( + url='http://example.com', + callback=NO_CALLBACK, + ) + self.assertIs(r5.callback, NO_CALLBACK) + def test_callback_and_errback_type(self): with self.assertRaises(TypeError): self.request_class('http://example.com', callback='a_function') @@ -288,6 +295,8 @@ class RequestTest(unittest.TestCase): callback='a_function', errback='a_function', ) + with self.assertRaises(TypeError): + self.request_class('http://example.com', errback=NO_CALLBACK) def test_from_curl(self): # Note: more curated tests regarding curl conversion are in diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4acd29bf7..83572e74f 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -33,10 +33,7 @@ from scrapy.utils.test import ( skip_if_no_boto, ) - -def _mocked_download_func(request, info): - response = request.meta.get('response') - return response() if callable(response) else response +from .test_pipeline_media import _mocked_download_func class FilesPipelineTestCase(unittest.TestCase): diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 6f5466191..3b39212bd 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -31,18 +31,13 @@ else: skip_pillow = None -def _mocked_download_func(request, info): - response = request.meta.get('response') - return response() if callable(response) else response - - class ImagesPipelineTestCase(unittest.TestCase): skip = skip_pillow def setUp(self): self.tempdir = mkdtemp() - self.pipeline = ImagesPipeline(self.tempdir, download_func=_mocked_download_func) + self.pipeline = ImagesPipeline(self.tempdir) def tearDown(self): rmtree(self.tempdir) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 0a94ae699..99fb424f4 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -9,6 +9,7 @@ from twisted.internet.defer import Deferred, inlineCallbacks from scrapy import signals from scrapy.http import Request, Response +from scrapy.http.request import NO_CALLBACK from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.pipelines.files import FileException @@ -29,6 +30,7 @@ else: def _mocked_download_func(request, info): + assert request.callback is NO_CALLBACK response = request.meta.get('response') return response() if callable(response) else response From a49346494201d3d7a9d017f16fd64fa2d9042b02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 19 Jan 2023 19:53:53 +0100 Subject: [PATCH 02/11] Update the screenshot pipeline code example --- docs/topics/item-pipeline.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 1672ccbcc..fa19d2f4c 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -191,6 +191,7 @@ item. import scrapy from itemadapter import ItemAdapter + from scrapy.http.request import NO_CALLBACK from scrapy.utils.defer import maybe_deferred_to_future @@ -204,8 +205,10 @@ item. adapter = ItemAdapter(item) encoded_item_url = quote(adapter["url"]) screenshot_url = self.SPLASH_URL.format(encoded_item_url) - request = scrapy.Request(screenshot_url) - response = await maybe_deferred_to_future(spider.crawler.engine.download(request, spider)) + request = scrapy.Request(screenshot_url, callback=NO_CALLBACK) + response = await maybe_deferred_to_future( + spider.crawler.engine.download(request, spider) + ) if response.status != 200: # Error happened, return item. From 5c1559f60e459a9678eecfe24c4fea06e272dbab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 19 Jan 2023 20:30:22 +0100 Subject: [PATCH 03/11] Address typing issues --- scrapy/http/request/__init__.py | 12 ++++++++++-- tox.ini | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index b57faf121..ea73781c8 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -5,7 +5,8 @@ requests in Scrapy. See documentation in docs/topics/request-response.rst """ import inspect -from typing import Callable, List, Optional, Tuple, Type, TypeVar, Union +from enum import Enum +from typing import Any, Callable, Final, List, Optional, Tuple, Type, TypeVar, Union from w3lib.url import safe_url_string @@ -20,6 +21,11 @@ from scrapy.utils.url import escape_ajax RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") + +# https://github.com/python/typing/issues/689#issuecomment-561425237 +class _NoCallback(Enum): + NO_CALLBACK = 0 + #: When assigned to the ``callback`` parameter of #: :class:`~scrapy.http.Request`, it indicates that the request it not meant to #: have a spider callback at all. @@ -29,7 +35,7 @@ RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") #: :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download #: middlewares handling such requests can treat them differently from requests #: intended for the :meth:`~scrapy.Spider.parse` callback. -NO_CALLBACK = object() +NO_CALLBACK: Final = _NoCallback.NO_CALLBACK class Request(object_ref): @@ -49,6 +55,8 @@ class Request(object_ref): Currently used by :meth:`Request.replace`, :meth:`Request.to_dict` and :func:`~scrapy.utils.request.request_from_dict`. """ + callback: Union[None, _NoCallback, Callable] + errback: Optional[Callable] def __init__( self, diff --git a/tox.ini b/tox.ini index 520d90303..076178d8e 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ # and then run "tox" from this directory. [tox] -envlist = security,flake8,py +envlist = security,flake8,typing,py minversion = 1.7.0 [testenv] From 4242ae405d9775098607a67612b5640a4daaf840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 19 Jan 2023 20:37:24 +0100 Subject: [PATCH 04/11] Restore Python 3.7 support --- scrapy/http/request/__init__.py | 4 +++- setup.py | 1 + tox.ini | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index ea73781c8..936afb007 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -6,8 +6,9 @@ See documentation in docs/topics/request-response.rst """ import inspect from enum import Enum -from typing import Any, Callable, Final, List, Optional, Tuple, Type, TypeVar, Union +from typing import Callable, List, Optional, Tuple, Type, TypeVar, Union +from typing_extensions import Final from w3lib.url import safe_url_string import scrapy @@ -26,6 +27,7 @@ RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") class _NoCallback(Enum): NO_CALLBACK = 0 + #: When assigned to the ``callback`` parameter of #: :class:`~scrapy.http.Request`, it indicates that the request it not meant to #: have a spider callback at all. diff --git a/setup.py b/setup.py index bdae28047..049c4f9a6 100644 --- a/setup.py +++ b/setup.py @@ -34,6 +34,7 @@ install_requires = [ 'packaging', 'tldextract', 'lxml>=4.3.0', + 'typing-extensions>=3.10.0.0', ] extras_require = {} cpython_dependencies = [ diff --git a/tox.ini b/tox.ini index 076178d8e..1ff5f7a4e 100644 --- a/tox.ini +++ b/tox.ini @@ -94,6 +94,7 @@ deps = w3lib==1.17.0 zope.interface==5.1.0 lxml==4.3.0 + typing-extensions==3.10.0.0 -rtests/requirements.txt # mitmproxy 4.0.4+ requires upgrading some of the pinned dependencies From 818d69fa003a14a2f2058e76258e6269d227efb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 20 Jan 2023 12:38:07 +0100 Subject: [PATCH 05/11] =?UTF-8?q?Fix=20typo:=20it=20=E2=86=92=20is?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrey Rakhmatullin --- scrapy/http/request/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 936afb007..302895781 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -29,7 +29,7 @@ class _NoCallback(Enum): #: When assigned to the ``callback`` parameter of -#: :class:`~scrapy.http.Request`, it indicates that the request it not meant to +#: :class:`~scrapy.http.Request`, it indicates that the request is not meant to #: have a spider callback at all. #: #: This value should be used by :ref:`components ` From c883a13006ad98f9f529fe130f7d717ad91f7afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 25 Jan 2023 17:43:10 +0100 Subject: [PATCH 06/11] Make the _set_xback condition more readable --- scrapy/http/request/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index a78ba5115..065f2daef 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -106,10 +106,10 @@ class Request(object_ref): self.flags = [] if flags is None else list(flags) def _set_xback(self, name: str, value: Optional[Callable]) -> None: - if ( - value is not None - and (name != "callback" or value is not NO_CALLBACK) - and not callable(value) + if not ( + callable(value) + or value is None + or (name == "callback" and value is NO_CALLBACK) ): raise TypeError(f"{name} must be a callable, got {type(value).__name__}") setattr(self, name, value) From 1f3e42897a6697f799d04a4a4320a7c16cbcd939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 25 Jan 2023 18:30:29 +0100 Subject: [PATCH 07/11] =?UTF-8?q?=5FNoCallback=20=E2=86=92=20NoCallbackTyp?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/http/request/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 924bd1d3e..068a4baa3 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -24,7 +24,7 @@ RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") # https://github.com/python/typing/issues/689#issuecomment-561425237 -class _NoCallback(Enum): +class NoCallbackType(Enum): NO_CALLBACK = 0 @@ -37,7 +37,7 @@ class _NoCallback(Enum): #: :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download #: middlewares handling such requests can treat them differently from requests #: intended for the :meth:`~scrapy.Spider.parse` callback. -NO_CALLBACK: Final = _NoCallback.NO_CALLBACK +NO_CALLBACK: Final = NoCallbackType.NO_CALLBACK class Request(object_ref): @@ -67,7 +67,7 @@ class Request(object_ref): Currently used by :meth:`Request.replace`, :meth:`Request.to_dict` and :func:`~scrapy.utils.request.request_from_dict`. """ - callback: Union[None, _NoCallback, Callable] + callback: Union[None, NoCallbackType, Callable] errback: Optional[Callable] def __init__( From f03b47db05e623189cf7719e647d18e8457494f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 27 Jan 2023 17:35:32 +0100 Subject: [PATCH 08/11] Make NO_CALLBACK a callable --- scrapy/http/request/__init__.py | 52 +++++++++++++++------------------ setup.py | 1 - tests/test_http_request.py | 8 +++-- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 068a4baa3..de13cf264 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -5,10 +5,8 @@ requests in Scrapy. See documentation in docs/topics/request-response.rst """ import inspect -from enum import Enum from typing import Callable, List, Optional, Tuple, Type, TypeVar, Union -from typing_extensions import Final from w3lib.url import safe_url_string import scrapy @@ -23,21 +21,22 @@ from scrapy.utils.url import escape_ajax RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") -# https://github.com/python/typing/issues/689#issuecomment-561425237 -class NoCallbackType(Enum): - NO_CALLBACK = 0 +def NO_CALLBACK(*args, **kwargs): + """When assigned to the ``callback`` parameter of + :class:`~scrapy.http.Request`, it indicates that the request is not meant + to have a spider callback at all. - -#: When assigned to the ``callback`` parameter of -#: :class:`~scrapy.http.Request`, it indicates that the request is not meant to -#: have a spider callback at all. -#: -#: This value should be used by :ref:`components ` -#: that create and handle their own requests, e.g. through -#: :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download -#: middlewares handling such requests can treat them differently from requests -#: intended for the :meth:`~scrapy.Spider.parse` callback. -NO_CALLBACK: Final = NoCallbackType.NO_CALLBACK + This value should be used by :ref:`components ` that + create and handle their own requests, e.g. through + :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download + middlewares handling such requests can treat them differently from requests + intended for the :meth:`~scrapy.Spider.parse` callback. + """ + raise RuntimeError( + "The NO_CALLBACK callback has been called. This is a special callback " + "value intended for requests whose callback is never meant to be " + "called." + ) class Request(object_ref): @@ -67,8 +66,6 @@ class Request(object_ref): Currently used by :meth:`Request.replace`, :meth:`Request.to_dict` and :func:`~scrapy.utils.request.request_from_dict`. """ - callback: Union[None, NoCallbackType, Callable] - errback: Optional[Callable] def __init__( self, @@ -94,8 +91,14 @@ class Request(object_ref): raise TypeError(f"Request priority not an integer: {priority!r}") self.priority = priority - self._set_xback("callback", callback) - self._set_xback("errback", errback) + if not (callable(callback) or callback is None): + raise TypeError( + f"callback must be a callable, got {type(callback).__name__}" + ) + if not (callable(errback) or errback is None): + raise TypeError(f"errback must be a callable, got {type(errback).__name__}") + self.callback = callback + self.errback = errback self.cookies = cookies or {} self.headers = Headers(headers or {}, encoding=encoding) @@ -105,15 +108,6 @@ class Request(object_ref): self._cb_kwargs = dict(cb_kwargs) if cb_kwargs else None self.flags = [] if flags is None else list(flags) - def _set_xback(self, name: str, value: Optional[Callable]) -> None: - if not ( - callable(value) - or value is None - or (name == "callback" and value is NO_CALLBACK) - ): - raise TypeError(f"{name} must be a callable, got {type(value).__name__}") - setattr(self, name, value) - @property def cb_kwargs(self) -> dict: if self._cb_kwargs is None: diff --git a/setup.py b/setup.py index 9150dac0b..f53334d4e 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,6 @@ install_requires = [ "packaging", "tldextract", "lxml>=4.3.0", - "typing-extensions>=3.10.0.0", ] extras_require = {} cpython_dependencies = [ diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 233a5f0b2..e800f427f 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -314,8 +314,10 @@ class RequestTest(unittest.TestCase): r5 = self.request_class( url="http://example.com", callback=NO_CALLBACK, + errback=NO_CALLBACK, ) self.assertIs(r5.callback, NO_CALLBACK) + self.assertIs(r5.errback, NO_CALLBACK) def test_callback_and_errback_type(self): with self.assertRaises(TypeError): @@ -328,8 +330,10 @@ class RequestTest(unittest.TestCase): callback="a_function", errback="a_function", ) - with self.assertRaises(TypeError): - self.request_class("http://example.com", errback=NO_CALLBACK) + + def test_no_callback(self): + with self.assertRaises(RuntimeError): + NO_CALLBACK() def test_from_curl(self): # Note: more curated tests regarding curl conversion are in From e1699479f6e48ce87dea1e6ed5661fea9ca7b1aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 30 Jan 2023 11:54:31 +0100 Subject: [PATCH 09/11] =?UTF-8?q?Fix=20typo:=20download=20middleware=20?= =?UTF-8?q?=E2=86=92=20downloader=20middleware?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrey Rakhmatullin --- scrapy/http/request/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 75dd2a74f..7afb28db5 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -27,7 +27,7 @@ def NO_CALLBACK(*args, **kwargs): This value should be used by :ref:`components ` that create and handle their own requests, e.g. through - :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download + :meth:`scrapy.core.engine.ExecutionEngine.download`, so that downloader middlewares handling such requests can treat them differently from requests intended for the :meth:`~scrapy.Spider.parse` callback. """ From 389fd99e79374bad73faf98424c97ac804eb1a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 30 Jan 2023 12:37:34 +0100 Subject: [PATCH 10/11] get_media_requests: support and encourage callback=NO_CALLBACK --- scrapy/pipelines/files.py | 3 ++- scrapy/pipelines/images.py | 3 ++- scrapy/pipelines/media.py | 9 ++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 01a9c41fe..91fc172b2 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -22,6 +22,7 @@ from twisted.internet import defer, threads from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request +from scrapy.http.request import NO_CALLBACK from scrapy.pipelines.media import MediaPipeline from scrapy.settings import Settings from scrapy.utils.boto import is_botocore_available @@ -517,7 +518,7 @@ class FilesPipeline(MediaPipeline): # Overridable Interface def get_media_requests(self, item, info): urls = ItemAdapter(item).get(self.files_urls_field, []) - return [Request(u) for u in urls] + return [Request(u, callback=NO_CALLBACK) for u in urls] def file_downloaded(self, response, request, info, *, item=None): path = self.file_path(request, response=response, info=info, item=item) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 0cfa5665a..9d18144ee 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -13,6 +13,7 @@ from itemadapter import ItemAdapter from scrapy.exceptions import DropItem, NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request +from scrapy.http.request import NO_CALLBACK from scrapy.pipelines.files import FileException, FilesPipeline # TODO: from scrapy.pipelines.media import MediaPipeline @@ -214,7 +215,7 @@ class ImagesPipeline(FilesPipeline): def get_media_requests(self, item, info): urls = ItemAdapter(item).get(self.images_urls_field, []) - return [Request(u) for u in urls] + return [Request(u, callback=NO_CALLBACK) for u in urls] def item_completed(self, results, item, info): with suppress(KeyError): diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 1e921f0b5..679035c5d 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -18,6 +18,10 @@ from scrapy.utils.log import failure_to_exc_info logger = logging.getLogger(__name__) +def _DUMMY_CALLBACK(response): + return response + + class MediaPipeline: LOG_FAILED_RESULTS = True @@ -91,7 +95,10 @@ class MediaPipeline: def _process_request(self, request, info, item): fp = self._fingerprinter.fingerprint(request) - cb = request.callback or (lambda _: _) + if not request.callback or request.callback is NO_CALLBACK: + cb = _DUMMY_CALLBACK + else: + cb = request.callback eb = request.errback request.callback = NO_CALLBACK request.errback = None From 78eaf0671bd50642f68d5b07bec3175298120a60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 30 Jan 2023 14:33:11 +0100 Subject: [PATCH 11/11] Remove typing-extensions from tox.ini --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index f2268b0f6..453c28c4c 100644 --- a/tox.ini +++ b/tox.ini @@ -94,7 +94,6 @@ deps = w3lib==1.17.0 zope.interface==5.1.0 lxml==4.3.0 - typing-extensions==3.10.0.0 -rtests/requirements.txt # mitmproxy 4.0.4+ requires upgrading some of the pinned dependencies