diff --git a/docs/topics/email.rst b/docs/topics/email.rst index d6a7ad354..8f7a2357a 100644 --- a/docs/topics/email.rst +++ b/docs/topics/email.rst @@ -27,13 +27,13 @@ the standard ``__init__`` method: mailer = MailSender() -Or you can instantiate it passing a Scrapy settings object, which will respect -the :ref:`settings `: +Or you can instantiate it passing a :class:`scrapy.Crawler` instance, which +will respect the :ref:`settings `: .. skip: start .. code-block:: python - mailer = MailSender.from_settings(settings) + mailer = MailSender.from_crawler(crawler) And here is how to use it to send an e-mail (without attachments): @@ -81,13 +81,13 @@ rest of the framework. :param smtpssl: enforce using a secure SSL connection :type smtpssl: bool - .. classmethod:: from_settings(settings) + .. classmethod:: from_crawler(crawler) - Instantiate using a Scrapy settings object, which will respect - :ref:`these Scrapy settings `. + Instantiate using a :class:`scrapy.Crawler` instance, which will + respect :ref:`these Scrapy settings `. - :param settings: the e-mail recipients - :type settings: :class:`scrapy.settings.Settings` object + :param crawler: the crawler + :type settings: :class:`scrapy.Crawler` object .. method:: send(to, subject, body, cc=None, attachs=(), mimetype='text/plain', charset=None) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 7c15b67e8..710e2e131 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -488,7 +488,7 @@ A request fingerprinter is a class that must implement the following method: :param request: request to fingerprint :type request: scrapy.http.Request -Additionally, it may also implement the following methods: +Additionally, it may also implement the following method: .. classmethod:: from_crawler(cls, crawler) :noindex: @@ -504,13 +504,6 @@ Additionally, it may also implement the following methods: :param crawler: crawler that uses this request fingerprinter :type crawler: :class:`~scrapy.crawler.Crawler` object -.. classmethod:: from_settings(cls, settings) - - If present, and ``from_crawler`` is not defined, this class method is called - to create a request fingerprinter instance from a - :class:`~scrapy.settings.Settings` object. It must return a new instance of - the request fingerprinter. - .. currentmodule:: scrapy.http The :meth:`fingerprint` method of the default request fingerprinter, diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index f80f832a7..8e17eab9a 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -21,6 +21,7 @@ from scrapy.core.downloader.tls import ( ScrapyClientTLSOptions, openssl_methods, ) +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.misc import build_from_crawler, load_object if TYPE_CHECKING: @@ -69,6 +70,31 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): method: int = SSL.SSLv23_METHOD, *args: Any, **kwargs: Any, + ) -> Self: + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls._from_settings(settings, method, *args, **kwargs) + + @classmethod + def from_crawler( + cls, + crawler: Crawler, + method: int = SSL.SSLv23_METHOD, + *args: Any, + **kwargs: Any, + ) -> Self: + return cls._from_settings(crawler.settings, method, *args, **kwargs) + + @classmethod + def _from_settings( + cls, + settings: BaseSettings, + method: int = SSL.SSLv23_METHOD, + *args: Any, + **kwargs: Any, ) -> Self: tls_verbose_logging: bool = settings.getbool( "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index d37d2741a..7b8eea135 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -1,9 +1,11 @@ from __future__ import annotations import logging +import warnings from pathlib import Path from typing import TYPE_CHECKING +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.job import job_dir from scrapy.utils.request import ( RequestFingerprinter, @@ -26,6 +28,15 @@ if TYPE_CHECKING: class BaseDupeFilter: @classmethod def from_settings(cls, settings: BaseSettings) -> Self: + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls() + + @classmethod + def from_crawler(cls, crawler: Crawler) -> Self: return cls() def request_seen(self, request: Request) -> bool: @@ -72,17 +83,31 @@ class RFPDupeFilter(BaseDupeFilter): *, fingerprinter: RequestFingerprinterProtocol | None = None, ) -> Self: - debug = settings.getbool("DUPEFILTER_DEBUG") - return cls(job_dir(settings), debug, fingerprinter=fingerprinter) + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls._from_settings(settings, fingerprinter=fingerprinter) @classmethod def from_crawler(cls, crawler: Crawler) -> Self: assert crawler.request_fingerprinter - return cls.from_settings( + return cls._from_settings( crawler.settings, fingerprinter=crawler.request_fingerprinter, ) + @classmethod + def _from_settings( + cls, + settings: BaseSettings, + *, + fingerprinter: RequestFingerprinterProtocol | None = None, + ) -> Self: + debug = settings.getbool("DUPEFILTER_DEBUG") + return cls(job_dir(settings), debug, fingerprinter=fingerprinter) + def request_seen(self, request: Request) -> bool: fp = self.request_fingerprint(request) if fp in self.fingerprints: diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index af06b7790..6a7704687 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -62,6 +62,11 @@ def build_storage( preargs: Iterable[Any] = (), **kwargs: Any, ) -> _StorageT: + warnings.warn( + "scrapy.extensions.feedexport.build_storage() is deprecated, call the builder directly.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) kwargs["feed_options"] = feed_options return builder(*preargs, uri, *args, **kwargs) @@ -248,8 +253,7 @@ class S3FeedStorage(BlockingFeedStorage): *, feed_options: dict[str, Any] | None = None, ) -> Self: - return build_storage( - cls, + return cls( uri, access_key=crawler.settings["AWS_ACCESS_KEY_ID"], secret_key=crawler.settings["AWS_SECRET_ACCESS_KEY"], @@ -323,10 +327,9 @@ class FTPFeedStorage(BlockingFeedStorage): *, feed_options: dict[str, Any] | None = None, ) -> Self: - return build_storage( - cls, + return cls( uri, - crawler.settings.getbool("FEED_STORAGE_FTP_ACTIVE"), + use_active_mode=crawler.settings.getbool("FEED_STORAGE_FTP_ACTIVE"), feed_options=feed_options, ) @@ -407,15 +410,12 @@ class FeedSlot: self.exporter.start_exporting() self._exporting = True - def _get_instance( - self, objcls: type[BaseItemExporter], *args: Any, **kwargs: Any - ) -> BaseItemExporter: - return build_from_crawler(objcls, self.crawler, *args, **kwargs) - def _get_exporter( self, file: IO[bytes], format: str, *args: Any, **kwargs: Any ) -> BaseItemExporter: - return self._get_instance(self.exporters[format], file, *args, **kwargs) + return build_from_crawler( + self.exporters[format], self.crawler, file, *args, **kwargs + ) def finish_exporting(self) -> None: if self._exporting: @@ -692,34 +692,8 @@ class FeedExporter: def _get_storage( self, uri: str, feed_options: dict[str, Any] ) -> FeedStorageProtocol: - """Fork of create_instance specific to feed storage classes - - It supports not passing the *feed_options* parameters to classes that - do not support it, and issuing a deprecation warning instead. - """ feedcls = self.storages.get(urlparse(uri).scheme, self.storages["file"]) - crawler = getattr(self, "crawler", None) - - def build_instance( - builder: type[FeedStorageProtocol], *preargs: Any - ) -> FeedStorageProtocol: - return build_storage( - builder, uri, feed_options=feed_options, preargs=preargs - ) - - instance: FeedStorageProtocol - if crawler and hasattr(feedcls, "from_crawler"): - instance = build_instance(feedcls.from_crawler, crawler) - method_name = "from_crawler" - elif hasattr(feedcls, "from_settings"): - instance = build_instance(feedcls.from_settings, self.settings) - method_name = "from_settings" - else: - instance = build_instance(feedcls) - method_name = "__new__" - if instance is None: - raise TypeError(f"{feedcls.__qualname__}.{method_name} returned None") - return instance + return build_from_crawler(feedcls, self.crawler, uri, feed_options=feed_options) def _get_uri_params( self, diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index 73d864d5d..d7f810107 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -48,7 +48,7 @@ class MemoryUsage: self.check_interval: float = crawler.settings.getfloat( "MEMUSAGE_CHECK_INTERVAL_SECONDS" ) - self.mail: MailSender = MailSender.from_settings(crawler.settings) + self.mail: MailSender = MailSender.from_crawler(crawler) crawler.signals.connect(self.engine_started, signal=signals.engine_started) crawler.signals.connect(self.engine_stopped, signal=signals.engine_stopped) diff --git a/scrapy/extensions/statsmailer.py b/scrapy/extensions/statsmailer.py index 600eebcf2..221628642 100644 --- a/scrapy/extensions/statsmailer.py +++ b/scrapy/extensions/statsmailer.py @@ -33,7 +33,7 @@ class StatsMailer: recipients: list[str] = crawler.settings.getlist("STATSMAILER_RCPTS") if not recipients: raise NotConfigured - mail: MailSender = MailSender.from_settings(crawler.settings) + mail: MailSender = MailSender.from_crawler(crawler) assert crawler.stats o = cls(crawler.stats, recipients, mail) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/mail.py b/scrapy/mail.py index 10dc7fed2..a3c642401 100644 --- a/scrapy/mail.py +++ b/scrapy/mail.py @@ -7,6 +7,7 @@ See documentation in docs/topics/email.rst from __future__ import annotations import logging +import warnings from email import encoders as Encoders from email.mime.base import MIMEBase from email.mime.multipart import MIMEMultipart @@ -19,6 +20,7 @@ from typing import IO, TYPE_CHECKING, Any from twisted.internet import ssl from twisted.internet.defer import Deferred +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.misc import arg_to_iter from scrapy.utils.python import to_bytes @@ -32,6 +34,7 @@ if TYPE_CHECKING: # typing.Self requires Python 3.11 from typing_extensions import Self + from scrapy.crawler import Crawler from scrapy.settings import BaseSettings @@ -72,6 +75,19 @@ class MailSender: @classmethod def from_settings(cls, settings: BaseSettings) -> Self: + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls._from_settings(settings) + + @classmethod + def from_crawler(cls, crawler: Crawler) -> Self: + return cls._from_settings(crawler.settings) + + @classmethod + def _from_settings(cls, settings: BaseSettings) -> Self: return cls( smtphost=settings["MAIL_HOST"], mailfrom=settings["MAIL_FROM"], diff --git a/scrapy/middleware.py b/scrapy/middleware.py index b6a427895..2b67dcd21 100644 --- a/scrapy/middleware.py +++ b/scrapy/middleware.py @@ -2,12 +2,13 @@ from __future__ import annotations import logging import pprint +import warnings from collections import defaultdict, deque from typing import TYPE_CHECKING, Any, TypeVar, cast -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.utils.defer import process_chain, process_parallel -from scrapy.utils.misc import build_from_crawler, build_from_settings, load_object +from scrapy.utils.misc import build_from_crawler, load_object if TYPE_CHECKING: from collections.abc import Callable, Iterable @@ -20,7 +21,7 @@ if TYPE_CHECKING: from scrapy import Spider from scrapy.crawler import Crawler - from scrapy.settings import Settings + from scrapy.settings import BaseSettings, Settings _P = ParamSpec("_P") @@ -50,8 +51,33 @@ class MiddlewareManager: def _get_mwlist_from_settings(cls, settings: Settings) -> list[Any]: raise NotImplementedError + @staticmethod + def _build_from_settings(objcls: type[_T], settings: BaseSettings) -> _T: + if hasattr(objcls, "from_settings"): + instance = objcls.from_settings(settings) # type: ignore[attr-defined] + method_name = "from_settings" + else: + instance = objcls() + method_name = "__new__" + if instance is None: + raise TypeError(f"{objcls.__qualname__}.{method_name} returned None") + return cast(_T, instance) + @classmethod def from_settings(cls, settings: Settings, crawler: Crawler | None = None) -> Self: + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls._from_settings(settings, crawler) + + @classmethod + def from_crawler(cls, crawler: Crawler) -> Self: + return cls._from_settings(crawler.settings, crawler) + + @classmethod + def _from_settings(cls, settings: Settings, crawler: Crawler | None = None) -> Self: mwlist = cls._get_mwlist_from_settings(settings) middlewares = [] enabled = [] @@ -61,7 +87,7 @@ class MiddlewareManager: if crawler is not None: mw = build_from_crawler(mwcls, crawler) else: - mw = build_from_settings(mwcls, settings) + mw = MiddlewareManager._build_from_settings(mwcls, settings) middlewares.append(mw) enabled.append(clspath) except NotConfigured as e: @@ -82,10 +108,6 @@ class MiddlewareManager: ) return cls(*middlewares) - @classmethod - def from_crawler(cls, crawler: Crawler) -> Self: - return cls.from_settings(crawler.settings, crawler) - def _add_middleware(self, mw: Any) -> None: if hasattr(mw, "open_spider"): self.methods["open_spider"].append(mw.open_spider) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 73cf37d28..bebf6039b 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -12,6 +12,7 @@ import hashlib import logging import mimetypes import time +import warnings from collections import defaultdict from contextlib import suppress from ftplib import FTP @@ -24,16 +25,17 @@ from itemadapter import ItemAdapter from twisted.internet.defer import Deferred, maybeDeferred from twisted.internet.threads import deferToThread -from scrapy.exceptions import IgnoreRequest, NotConfigured +from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK from scrapy.pipelines.media import FileInfo, FileInfoOrError, MediaPipeline -from scrapy.settings import Settings +from scrapy.settings import BaseSettings, Settings from scrapy.utils.boto import is_botocore_available from scrapy.utils.datatypes import CaseInsensitiveDict +from scrapy.utils.deprecate import method_is_overridden from scrapy.utils.ftp import ftp_store_file from scrapy.utils.log import failure_to_exc_info -from scrapy.utils.python import to_bytes +from scrapy.utils.python import get_func_args, global_object_name, to_bytes from scrapy.utils.request import referer_str if TYPE_CHECKING: @@ -46,6 +48,7 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy import Spider + from scrapy.crawler import Crawler logger = logging.getLogger(__name__) @@ -442,12 +445,24 @@ class FilesPipeline(MediaPipeline): store_uri: str | PathLike[str], download_func: Callable[[Request, Spider], Response] | None = None, settings: Settings | dict[str, Any] | None = None, + *, + crawler: Crawler | None = None, ): store_uri = _to_string(store_uri) if not store_uri: raise NotConfigured - if isinstance(settings, dict) or settings is None: + if crawler is not None: + if settings is not None: + warnings.warn( + f"FilesPipeline.__init__() was called with a crawler instance and a settings instance" + f" when creating {global_object_name(self.__class__)}. The settings instance will be ignored" + f" and crawler.settings will be used. The settings argument will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + settings = crawler.settings + elif isinstance(settings, dict) or settings is None: settings = Settings(settings) cls_name = "FilesPipeline" self.store: FilesStoreProtocol = self._get_store(store_uri) @@ -466,10 +481,54 @@ class FilesPipeline(MediaPipeline): resolve("FILES_RESULT_FIELD"), self.FILES_RESULT_FIELD ) - super().__init__(download_func=download_func, settings=settings) + super().__init__( + download_func=download_func, + settings=settings if not crawler else None, + crawler=crawler, + ) @classmethod def from_settings(cls, settings: Settings) -> Self: + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls._from_settings(settings, None) + + @classmethod + def from_crawler(cls, crawler: Crawler) -> Self: + if method_is_overridden(cls, FilesPipeline, "from_settings"): + warnings.warn( + f"{global_object_name(cls)} overrides FilesPipeline.from_settings()." + f" This method is deprecated and won't be called in future Scrapy versions," + f" please update your code so that it overrides from_crawler() instead.", + category=ScrapyDeprecationWarning, + ) + o = cls.from_settings(crawler.settings) + o._finish_init(crawler) + return o + return cls._from_settings(crawler.settings, crawler) + + @classmethod + def _from_settings(cls, settings: Settings, crawler: Crawler | None) -> Self: + cls._update_stores(settings) + store_uri = settings["FILES_STORE"] + if "crawler" in get_func_args(cls.__init__): + o = cls(store_uri, crawler=crawler) + else: + o = cls(store_uri, settings=settings) + if crawler: + o._finish_init(crawler) + warnings.warn( + f"{global_object_name(cls)}.__init__() doesn't take a crawler argument." + " This is deprecated and the argument will be required in future Scrapy versions.", + category=ScrapyDeprecationWarning, + ) + return o + + @classmethod + def _update_stores(cls, settings: BaseSettings) -> None: s3store: type[S3FilesStore] = cast(type[S3FilesStore], cls.STORE_SCHEMES["s3"]) s3store.AWS_ACCESS_KEY_ID = settings["AWS_ACCESS_KEY_ID"] s3store.AWS_SECRET_ACCESS_KEY = settings["AWS_SECRET_ACCESS_KEY"] @@ -493,9 +552,6 @@ class FilesPipeline(MediaPipeline): ftp_store.FTP_PASSWORD = settings["FTP_PASSWORD"] ftp_store.USE_ACTIVE_MODE = settings.getbool("FEED_STORAGE_FTP_ACTIVE") - store_uri = settings["FILES_STORE"] - return cls(store_uri, settings=settings) - def _get_store(self, uri: str) -> FilesStoreProtocol: if Path(uri).is_absolute(): # to support win32 paths like: C:\\some\dir scheme = "file" diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 2c4c9376e..e86e7c493 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -8,25 +8,19 @@ from __future__ import annotations import functools import hashlib +import warnings from contextlib import suppress from io import BytesIO -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any from itemadapter import ItemAdapter -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK -from scrapy.pipelines.files import ( - FileException, - FilesPipeline, - FTPFilesStore, - GCSFilesStore, - S3FilesStore, - _md5sum, -) +from scrapy.pipelines.files import FileException, FilesPipeline, _md5sum from scrapy.settings import Settings -from scrapy.utils.python import to_bytes +from scrapy.utils.python import get_func_args, global_object_name, to_bytes if TYPE_CHECKING: from collections.abc import Callable, Iterable @@ -38,6 +32,7 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy import Spider + from scrapy.crawler import Crawler from scrapy.pipelines.media import FileInfoOrError, MediaPipeline @@ -64,6 +59,8 @@ class ImagesPipeline(FilesPipeline): store_uri: str | PathLike[str], download_func: Callable[[Request, Spider], Response] | None = None, settings: Settings | dict[str, Any] | None = None, + *, + crawler: Crawler | None = None, ): try: from PIL import Image @@ -74,9 +71,24 @@ class ImagesPipeline(FilesPipeline): "ImagesPipeline requires installing Pillow 4.0.0 or later" ) - super().__init__(store_uri, settings=settings, download_func=download_func) + super().__init__( + store_uri, + settings=settings if not crawler else None, + download_func=download_func, + crawler=crawler, + ) - if isinstance(settings, dict) or settings is None: + if crawler is not None: + if settings is not None: + warnings.warn( + f"ImagesPipeline.__init__() was called with a crawler instance and a settings instance" + f" when creating {global_object_name(self.__class__)}. The settings instance will be ignored" + f" and crawler.settings will be used. The settings argument will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + settings = crawler.settings + elif isinstance(settings, dict) or settings is None: settings = Settings(settings) resolve = functools.partial( @@ -108,32 +120,21 @@ class ImagesPipeline(FilesPipeline): ) @classmethod - def from_settings(cls, settings: Settings) -> Self: - s3store: type[S3FilesStore] = cast(type[S3FilesStore], cls.STORE_SCHEMES["s3"]) - s3store.AWS_ACCESS_KEY_ID = settings["AWS_ACCESS_KEY_ID"] - s3store.AWS_SECRET_ACCESS_KEY = settings["AWS_SECRET_ACCESS_KEY"] - s3store.AWS_SESSION_TOKEN = settings["AWS_SESSION_TOKEN"] - s3store.AWS_ENDPOINT_URL = settings["AWS_ENDPOINT_URL"] - s3store.AWS_REGION_NAME = settings["AWS_REGION_NAME"] - s3store.AWS_USE_SSL = settings["AWS_USE_SSL"] - s3store.AWS_VERIFY = settings["AWS_VERIFY"] - s3store.POLICY = settings["IMAGES_STORE_S3_ACL"] - - gcs_store: type[GCSFilesStore] = cast( - type[GCSFilesStore], cls.STORE_SCHEMES["gs"] - ) - gcs_store.GCS_PROJECT_ID = settings["GCS_PROJECT_ID"] - gcs_store.POLICY = settings["IMAGES_STORE_GCS_ACL"] or None - - ftp_store: type[FTPFilesStore] = cast( - type[FTPFilesStore], cls.STORE_SCHEMES["ftp"] - ) - ftp_store.FTP_USERNAME = settings["FTP_USER"] - ftp_store.FTP_PASSWORD = settings["FTP_PASSWORD"] - ftp_store.USE_ACTIVE_MODE = settings.getbool("FEED_STORAGE_FTP_ACTIVE") - + def _from_settings(cls, settings: Settings, crawler: Crawler | None) -> Self: + cls._update_stores(settings) store_uri = settings["IMAGES_STORE"] - return cls(store_uri, settings=settings) + if "crawler" in get_func_args(cls.__init__): + o = cls(store_uri, crawler=crawler) + else: + o = cls(store_uri, settings=settings) + if crawler: + o._finish_init(crawler) + warnings.warn( + f"{global_object_name(cls)}.__init__() doesn't take a crawler argument." + " This is deprecated and the argument will be required in future Scrapy versions.", + category=ScrapyDeprecationWarning, + ) + return o def file_downloaded( self, diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index b10ec147b..6d7808c31 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -2,6 +2,7 @@ from __future__ import annotations import functools import logging +import warnings from abc import ABC, abstractmethod from collections import defaultdict from typing import ( @@ -20,12 +21,14 @@ from twisted.internet.defer import Deferred, DeferredList from twisted.python.failure import Failure from twisted.python.versions import Version +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http.request import NO_CALLBACK, Request from scrapy.settings import Settings from scrapy.utils.datatypes import SequenceExclude from scrapy.utils.defer import defer_result, mustbe_deferred from scrapy.utils.log import failure_to_exc_info from scrapy.utils.misc import arg_to_iter +from scrapy.utils.python import get_func_args, global_object_name if TYPE_CHECKING: from collections.abc import Callable @@ -38,7 +41,6 @@ if TYPE_CHECKING: from scrapy.http import Response from scrapy.utils.request import RequestFingerprinter - _T = TypeVar("_T") @@ -51,13 +53,13 @@ class FileInfo(TypedDict): FileInfoOrError = Union[tuple[Literal[True], FileInfo], tuple[Literal[False], Failure]] - logger = logging.getLogger(__name__) class MediaPipeline(ABC): crawler: Crawler _fingerprinter: RequestFingerprinter + _modern_init = False LOG_FAILED_RESULTS: bool = True @@ -74,10 +76,22 @@ class MediaPipeline(ABC): self, download_func: Callable[[Request, Spider], Response] | None = None, settings: Settings | dict[str, Any] | None = None, + *, + crawler: Crawler | None = None, ): self.download_func = download_func - if isinstance(settings, dict) or settings is None: + if crawler is not None: + if settings is not None: + warnings.warn( + f"MediaPipeline.__init__() was called with a crawler instance and a settings instance" + f" when creating {global_object_name(self.__class__)}. The settings instance will be ignored" + f" and crawler.settings will be used. The settings argument will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + settings = crawler.settings + elif isinstance(settings, dict) or settings is None: settings = Settings(settings) resolve = functools.partial( self._key_for_pipe, base_class_name="MediaPipeline", settings=settings @@ -87,6 +101,27 @@ class MediaPipeline(ABC): ) self._handle_statuses(self.allow_redirects) + if crawler: + self._finish_init(crawler) + self._modern_init = True + else: + warnings.warn( + f"MediaPipeline.__init__() was called without the crawler argument" + f" when creating {global_object_name(self.__class__)}." + f" This is deprecated and the argument will be required in future Scrapy versions.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + + def _finish_init(self, crawler: Crawler) -> None: + # This was done in from_crawler() before 2.12, now it's done in __init__() + # if the crawler was passed to it and may be needed to be called in other + # deprecated code paths explicitly too. After the crawler argument of __init__() + # becomes mandatory this should be inlined there. + self.crawler = crawler + assert crawler.request_fingerprinter + self._fingerprinter = crawler.request_fingerprinter + def _handle_statuses(self, allow_redirects: bool) -> None: self.handle_httpstatus_list = None if allow_redirects: @@ -112,13 +147,19 @@ class MediaPipeline(ABC): @classmethod def from_crawler(cls, crawler: Crawler) -> Self: pipe: Self - try: + if hasattr(cls, "from_settings"): pipe = cls.from_settings(crawler.settings) # type: ignore[attr-defined] - except AttributeError: + elif "crawler" in get_func_args(cls.__init__): + pipe = cls(crawler=crawler) + else: pipe = cls() - pipe.crawler = crawler - assert crawler.request_fingerprinter - pipe._fingerprinter = crawler.request_fingerprinter + warnings.warn( + f"{global_object_name(cls)}.__init__() doesn't take a crawler argument." + " This is deprecated and the argument will be required in future Scrapy versions.", + category=ScrapyDeprecationWarning, + ) + if not pipe._modern_init: + pipe._finish_init(crawler) return pipe def open_spider(self, spider: Spider) -> None: diff --git a/scrapy/spidermiddlewares/urllength.py b/scrapy/spidermiddlewares/urllength.py index 191adb6cd..a1cd1bb7c 100644 --- a/scrapy/spidermiddlewares/urllength.py +++ b/scrapy/spidermiddlewares/urllength.py @@ -7,9 +7,10 @@ See documentation in docs/topics/spider-middleware.rst from __future__ import annotations import logging +import warnings from typing import TYPE_CHECKING, Any -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request, Response if TYPE_CHECKING: @@ -19,6 +20,7 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy import Spider + from scrapy.crawler import Crawler from scrapy.settings import BaseSettings @@ -31,6 +33,19 @@ class UrlLengthMiddleware: @classmethod def from_settings(cls, settings: BaseSettings) -> Self: + warnings.warn( + f"{cls.__name__}.from_settings() is deprecated, use from_crawler() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + return cls._from_settings(settings) + + @classmethod + def from_crawler(cls, crawler: Crawler) -> Self: + return cls._from_settings(crawler.settings) + + @classmethod + def _from_settings(cls, settings: BaseSettings) -> Self: maxlength = settings.getint("URLLENGTH_LIMIT") if not maxlength: raise NotConfigured diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index 12c09839f..a408a205d 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -26,7 +26,6 @@ if TYPE_CHECKING: from scrapy import Spider from scrapy.crawler import Crawler - from scrapy.settings import BaseSettings _ITERABLE_SINGLE_VALUES = dict, Item, str, bytes @@ -150,7 +149,7 @@ def create_instance(objcls, settings, crawler, *args, **kwargs): """ warnings.warn( "The create_instance() function is deprecated. " - "Please use build_from_crawler() or build_from_settings() instead.", + "Please use build_from_crawler() instead.", category=ScrapyDeprecationWarning, stacklevel=2, ) @@ -176,7 +175,7 @@ def create_instance(objcls, settings, crawler, *args, **kwargs): def build_from_crawler( objcls: type[T], crawler: Crawler, /, *args: Any, **kwargs: Any ) -> T: - """Construct a class instance using its ``from_crawler`` constructor. + """Construct a class instance using its ``from_crawler`` or ``from_settings`` constructor. ``*args`` and ``**kwargs`` are forwarded to the constructor. @@ -186,6 +185,14 @@ def build_from_crawler( instance = objcls.from_crawler(crawler, *args, **kwargs) # type: ignore[attr-defined] method_name = "from_crawler" elif hasattr(objcls, "from_settings"): + warnings.warn( + f"{objcls.__qualname__} has from_settings() but not from_crawler()." + " This is deprecated and calling from_settings() will be removed in a future" + " Scrapy version. You can implement a simple from_crawler() that calls" + " from_settings() with crawler.settings.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) instance = objcls.from_settings(crawler.settings, *args, **kwargs) # type: ignore[attr-defined] method_name = "from_settings" else: @@ -196,26 +203,6 @@ def build_from_crawler( return cast(T, instance) -def build_from_settings( - objcls: type[T], settings: BaseSettings, /, *args: Any, **kwargs: Any -) -> T: - """Construct a class instance using its ``from_settings`` constructor. - - ``*args`` and ``**kwargs`` are forwarded to the constructor. - - Raises ``TypeError`` if the resulting instance is ``None``. - """ - if hasattr(objcls, "from_settings"): - instance = objcls.from_settings(settings, *args, **kwargs) # type: ignore[attr-defined] - method_name = "from_settings" - else: - instance = objcls(*args, **kwargs) - method_name = "__new__" - if instance is None: - raise TypeError(f"{objcls.__qualname__}.{method_name} returned None") - return cast(T, instance) - - @contextmanager def set_environ(**kwargs: str) -> Iterator[None]: """Temporarily set environment variables inside the context manager and diff --git a/tests/test_dupefilters.py b/tests/test_dupefilters.py index 9ba8bd64f..4fd648f48 100644 --- a/tests/test_dupefilters.py +++ b/tests/test_dupefilters.py @@ -33,14 +33,6 @@ class FromCrawlerRFPDupeFilter(RFPDupeFilter): return df -class FromSettingsRFPDupeFilter(RFPDupeFilter): - @classmethod - def from_settings(cls, settings, *, fingerprinter=None): - df = super().from_settings(settings, fingerprinter=fingerprinter) - df.method = "from_settings" - return df - - class DirectDupeFilter: method = "n/a" @@ -56,16 +48,6 @@ class RFPDupeFilterTest(unittest.TestCase): self.assertTrue(scheduler.df.debug) self.assertEqual(scheduler.df.method, "from_crawler") - def test_df_from_settings_scheduler(self): - settings = { - "DUPEFILTER_DEBUG": True, - "DUPEFILTER_CLASS": FromSettingsRFPDupeFilter, - } - crawler = get_crawler(settings_dict=settings) - scheduler = Scheduler.from_crawler(crawler) - self.assertTrue(scheduler.df.debug) - self.assertEqual(scheduler.df.method, "from_settings") - def test_df_direct_scheduler(self): settings = { "DUPEFILTER_CLASS": DirectDupeFilter, diff --git a/tests/test_middleware.py b/tests/test_middleware.py index a42c7b3d1..3a1cf19ad 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -2,7 +2,7 @@ from twisted.trial import unittest from scrapy.exceptions import NotConfigured from scrapy.middleware import MiddlewareManager -from scrapy.settings import Settings +from scrapy.utils.test import get_crawler class M1: @@ -23,8 +23,6 @@ class M2: def close_spider(self, spider): pass - pass - class M3: def process(self, response, request, spider): @@ -83,7 +81,7 @@ class MiddlewareManagerTest(unittest.TestCase): self.assertEqual(mwman.middlewares, (m1, m2, m3)) def test_enabled_from_settings(self): - settings = Settings() - mwman = TestMiddlewareManager.from_settings(settings) + crawler = get_crawler() + mwman = TestMiddlewareManager.from_crawler(crawler) classes = [x.__class__ for x in mwman.middlewares] self.assertEqual(classes, [M1, M3]) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 80bb9e939..96a2c42b7 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -2,6 +2,7 @@ import dataclasses import os import random import time +import warnings from datetime import datetime from io import BytesIO from pathlib import Path @@ -25,7 +26,6 @@ from scrapy.pipelines.files import ( GCSFilesStore, S3FilesStore, ) -from scrapy.settings import Settings from scrapy.utils.test import ( assert_gcs_environ, get_crawler, @@ -217,8 +217,8 @@ class FilesPipelineTestCase(unittest.TestCase): def file_path(self, request, response=None, info=None, item=None): return f'full/{item.get("path")}' - file_path = CustomFilesPipeline.from_settings( - Settings({"FILES_STORE": self.tempdir}) + file_path = CustomFilesPipeline.from_crawler( + get_crawler(None, {"FILES_STORE": self.tempdir}) ).file_path item = {"path": "path-to-store-file"} request = Request("http://example.com") @@ -235,7 +235,9 @@ class FilesPipelineTestCaseFieldsMixin: def test_item_fields_default(self): url = "http://www.example.com/files/1.txt" item = self.item_class(name="item1", file_urls=[url]) - pipeline = FilesPipeline.from_settings(Settings({"FILES_STORE": self.tempdir})) + pipeline = FilesPipeline.from_crawler( + get_crawler(None, {"FILES_STORE": self.tempdir}) + ) requests = list(pipeline.get_media_requests(item, None)) self.assertEqual(requests[0].url, url) results = [(True, {"url": url})] @@ -247,13 +249,14 @@ class FilesPipelineTestCaseFieldsMixin: def test_item_fields_override_settings(self): url = "http://www.example.com/files/1.txt" item = self.item_class(name="item1", custom_file_urls=[url]) - pipeline = FilesPipeline.from_settings( - Settings( + pipeline = FilesPipeline.from_crawler( + get_crawler( + None, { "FILES_STORE": self.tempdir, "FILES_URLS_FIELD": "custom_file_urls", "FILES_RESULT_FIELD": "custom_files", - } + }, ) ) requests = list(pipeline.get_media_requests(item, None)) @@ -371,8 +374,10 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): different settings. """ custom_settings = self._generate_fake_settings() - another_pipeline = FilesPipeline.from_settings(Settings(custom_settings)) - one_pipeline = FilesPipeline(self.tempdir) + another_pipeline = FilesPipeline.from_crawler( + get_crawler(None, custom_settings) + ) + one_pipeline = FilesPipeline(self.tempdir, crawler=get_crawler(None)) for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: default_value = self.default_cls_settings[pipe_attr] self.assertEqual(getattr(one_pipeline, pipe_attr), default_value) @@ -385,7 +390,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): If subclasses override class attributes and there are no special settings those values should be kept. """ pipe_cls = self._generate_fake_pipeline() - pipe = pipe_cls.from_settings(Settings({"FILES_STORE": self.tempdir})) + pipe = pipe_cls.from_crawler(get_crawler(None, {"FILES_STORE": self.tempdir})) for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: custom_value = getattr(pipe, pipe_ins_attr) self.assertNotEqual(custom_value, self.default_cls_settings[pipe_attr]) @@ -398,7 +403,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): """ pipeline_cls = self._generate_fake_pipeline() settings = self._generate_fake_settings() - pipeline = pipeline_cls.from_settings(Settings(settings)) + pipeline = pipeline_cls.from_crawler(get_crawler(None, settings)) for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: value = getattr(pipeline, pipe_ins_attr) setting_value = settings.get(settings_attr) @@ -414,8 +419,8 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): class UserDefinedFilesPipeline(FilesPipeline): pass - user_pipeline = UserDefinedFilesPipeline.from_settings( - Settings({"FILES_STORE": self.tempdir}) + user_pipeline = UserDefinedFilesPipeline.from_crawler( + get_crawler(None, {"FILES_STORE": self.tempdir}) ) for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: # Values from settings for custom pipeline should be set on pipeline instance. @@ -433,7 +438,9 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): prefix = UserDefinedFilesPipeline.__name__.upper() settings = self._generate_fake_settings(prefix=prefix) - user_pipeline = UserDefinedFilesPipeline.from_settings(Settings(settings)) + user_pipeline = UserDefinedFilesPipeline.from_crawler( + get_crawler(None, settings) + ) for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: # Values from settings for custom pipeline should be set on pipeline instance. custom_value = settings.get(prefix + "_" + settings_attr) @@ -448,7 +455,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): pipeline_cls = self._generate_fake_pipeline() prefix = pipeline_cls.__name__.upper() settings = self._generate_fake_settings(prefix=prefix) - user_pipeline = pipeline_cls.from_settings(Settings(settings)) + user_pipeline = pipeline_cls.from_crawler(get_crawler(None, settings)) for ( pipe_cls_attr, settings_attr, @@ -463,8 +470,8 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): DEFAULT_FILES_RESULT_FIELD = "this" DEFAULT_FILES_URLS_FIELD = "that" - pipeline = UserDefinedFilesPipeline.from_settings( - Settings({"FILES_STORE": self.tempdir}) + pipeline = UserDefinedFilesPipeline.from_crawler( + get_crawler(None, {"FILES_STORE": self.tempdir}) ) self.assertEqual( pipeline.files_result_field, @@ -484,7 +491,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): class UserPipe(FilesPipeline): pass - pipeline_cls = UserPipe.from_settings(Settings(settings)) + pipeline_cls = UserPipe.from_crawler(get_crawler(None, settings)) for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: expected_value = settings.get(settings_attr) @@ -495,8 +502,8 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): def file_path(self, request, response=None, info=None, *, item=None): return Path("subdir") / Path(request.url).name - pipeline = CustomFilesPipelineWithPathLikeDir.from_settings( - Settings({"FILES_STORE": Path("./Temp")}) + pipeline = CustomFilesPipelineWithPathLikeDir.from_crawler( + get_crawler(None, {"FILES_STORE": Path("./Temp")}) ) request = Request("http://example.com/image01.jpg") self.assertEqual(pipeline.file_path(request), Path("subdir/image01.jpg")) @@ -686,3 +693,75 @@ def _prepare_request_object(item_url, flags=None): item_url, meta={"response": Response(item_url, status=200, body=b"data", flags=flags)}, ) + + +# this is separate from the one in test_pipeline_media.py to specifically test FilesPipeline subclasses +class BuildFromCrawlerTestCase(unittest.TestCase): + def setUp(self): + self.tempdir = mkdtemp() + self.crawler = get_crawler(None, {"FILES_STORE": self.tempdir}) + + def tearDown(self): + rmtree(self.tempdir) + + def test_simple(self): + class Pipeline(FilesPipeline): + pass + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 0) + assert pipe.store + + def test_has_old_init(self): + class Pipeline(FilesPipeline): + def __init__(self, store_uri, download_func=None, settings=None): + super().__init__(store_uri, download_func, settings) + self._init_called = True + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 2) + assert pipe._init_called + + def test_has_from_settings(self): + class Pipeline(FilesPipeline): + _from_settings_called = False + + @classmethod + def from_settings(cls, settings): + o = super().from_settings(settings) + o._from_settings_called = True + return o + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 3) + assert pipe.store + assert pipe._from_settings_called + + def test_has_from_crawler_and_init(self): + class Pipeline(FilesPipeline): + _from_crawler_called = False + + @classmethod + def from_crawler(cls, crawler): + settings = crawler.settings + store_uri = settings["FILES_STORE"] + o = cls(store_uri, crawler=crawler) + o._from_crawler_called = True + return o + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 0) + assert pipe.store + assert pipe._from_crawler_called diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index dfeead999..3ffef4102 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -13,7 +13,7 @@ from twisted.trial import unittest from scrapy.http import Request, Response from scrapy.item import Field, Item from scrapy.pipelines.images import ImageException, ImagesPipeline -from scrapy.settings import Settings +from scrapy.utils.test import get_crawler skip_pillow: str | None try: @@ -33,7 +33,8 @@ class ImagesPipelineTestCase(unittest.TestCase): def setUp(self): self.tempdir = mkdtemp() - self.pipeline = ImagesPipeline(self.tempdir) + crawler = get_crawler() + self.pipeline = ImagesPipeline(self.tempdir, crawler=crawler) def tearDown(self): rmtree(self.tempdir) @@ -123,8 +124,8 @@ class ImagesPipelineTestCase(unittest.TestCase): ): return f"thumb/{thumb_id}/{item.get('path')}" - thumb_path = CustomImagesPipeline.from_settings( - Settings({"IMAGES_STORE": self.tempdir}) + thumb_path = CustomImagesPipeline.from_crawler( + get_crawler(None, {"IMAGES_STORE": self.tempdir}) ).thumb_path item = {"path": "path-to-store-file"} request = Request("http://example.com") @@ -218,8 +219,8 @@ class ImagesPipelineTestCaseFieldsMixin: def test_item_fields_default(self): url = "http://www.example.com/images/1.jpg" item = self.item_class(name="item1", image_urls=[url]) - pipeline = ImagesPipeline.from_settings( - Settings({"IMAGES_STORE": "s3://example/images/"}) + pipeline = ImagesPipeline.from_crawler( + get_crawler(None, {"IMAGES_STORE": "s3://example/images/"}) ) requests = list(pipeline.get_media_requests(item, None)) self.assertEqual(requests[0].url, url) @@ -232,13 +233,14 @@ class ImagesPipelineTestCaseFieldsMixin: def test_item_fields_override_settings(self): url = "http://www.example.com/images/1.jpg" item = self.item_class(name="item1", custom_image_urls=[url]) - pipeline = ImagesPipeline.from_settings( - Settings( + pipeline = ImagesPipeline.from_crawler( + get_crawler( + None, { "IMAGES_STORE": "s3://example/images/", "IMAGES_URLS_FIELD": "custom_image_urls", "IMAGES_RESULT_FIELD": "custom_images", - } + }, ) ) requests = list(pipeline.get_media_requests(item, None)) @@ -389,9 +391,8 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): have different settings. """ custom_settings = self._generate_fake_settings() - default_settings = Settings() - default_sts_pipe = ImagesPipeline(self.tempdir, settings=default_settings) - user_sts_pipe = ImagesPipeline.from_settings(Settings(custom_settings)) + default_sts_pipe = ImagesPipeline(self.tempdir, crawler=get_crawler(None)) + user_sts_pipe = ImagesPipeline.from_crawler(get_crawler(None, custom_settings)) for pipe_attr, settings_attr in self.img_cls_attribute_names: expected_default_value = self.default_pipeline_settings.get(pipe_attr) custom_value = custom_settings.get(settings_attr) @@ -407,7 +408,9 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): from class attributes. """ pipeline_cls = self._generate_fake_pipeline_subclass() - pipeline = pipeline_cls.from_settings(Settings({"IMAGES_STORE": self.tempdir})) + pipeline = pipeline_cls.from_crawler( + get_crawler(None, {"IMAGES_STORE": self.tempdir}) + ) for pipe_attr, settings_attr in self.img_cls_attribute_names: # Instance attribute (lowercase) must be equal to class attribute (uppercase). attr_value = getattr(pipeline, pipe_attr.lower()) @@ -421,7 +424,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): """ pipeline_cls = self._generate_fake_pipeline_subclass() settings = self._generate_fake_settings() - pipeline = pipeline_cls.from_settings(Settings(settings)) + pipeline = pipeline_cls.from_crawler(get_crawler(None, settings)) for pipe_attr, settings_attr in self.img_cls_attribute_names: # Instance attribute (lowercase) must be equal to # value defined in settings. @@ -439,8 +442,8 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): class UserDefinedImagePipeline(ImagesPipeline): pass - user_pipeline = UserDefinedImagePipeline.from_settings( - Settings({"IMAGES_STORE": self.tempdir}) + user_pipeline = UserDefinedImagePipeline.from_crawler( + get_crawler(None, {"IMAGES_STORE": self.tempdir}) ) for pipe_attr, settings_attr in self.img_cls_attribute_names: # Values from settings for custom pipeline should be set on pipeline instance. @@ -458,7 +461,9 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): prefix = UserDefinedImagePipeline.__name__.upper() settings = self._generate_fake_settings(prefix=prefix) - user_pipeline = UserDefinedImagePipeline.from_settings(Settings(settings)) + user_pipeline = UserDefinedImagePipeline.from_crawler( + get_crawler(None, settings) + ) for pipe_attr, settings_attr in self.img_cls_attribute_names: # Values from settings for custom pipeline should be set on pipeline instance. custom_value = settings.get(prefix + "_" + settings_attr) @@ -473,7 +478,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): pipeline_cls = self._generate_fake_pipeline_subclass() prefix = pipeline_cls.__name__.upper() settings = self._generate_fake_settings(prefix=prefix) - user_pipeline = pipeline_cls.from_settings(Settings(settings)) + user_pipeline = pipeline_cls.from_crawler(get_crawler(None, settings)) for pipe_attr, settings_attr in self.img_cls_attribute_names: custom_value = settings.get(prefix + "_" + settings_attr) self.assertNotEqual(custom_value, self.default_pipeline_settings[pipe_attr]) @@ -484,8 +489,8 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): DEFAULT_IMAGES_URLS_FIELD = "something" DEFAULT_IMAGES_RESULT_FIELD = "something_else" - pipeline = UserDefinedImagePipeline.from_settings( - Settings({"IMAGES_STORE": self.tempdir}) + pipeline = UserDefinedImagePipeline.from_crawler( + get_crawler(None, {"IMAGES_STORE": self.tempdir}) ) self.assertEqual( pipeline.images_result_field, @@ -506,7 +511,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): class UserPipe(ImagesPipeline): pass - pipeline_cls = UserPipe.from_settings(Settings(settings)) + pipeline_cls = UserPipe.from_crawler(get_crawler(None, settings)) for pipe_attr, settings_attr in self.img_cls_attribute_names: expected_value = settings.get(settings_attr) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index c979e45d7..58a2d3678 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -1,5 +1,7 @@ from __future__ import annotations +import warnings + from testfixtures import LogCapture from twisted.internet import reactor from twisted.internet.defer import Deferred, inlineCallbacks @@ -11,7 +13,6 @@ from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK from scrapy.pipelines.files import FileException from scrapy.pipelines.media import MediaPipeline -from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.utils.log import failure_to_exc_info from scrapy.utils.signal import disconnect_all @@ -175,8 +176,8 @@ class BaseMediaPipelineTestCase(unittest.TestCase): class MockedMediaPipeline(UserDefinedPipeline): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, *args, crawler=None, **kwargs): + super().__init__(*args, crawler=crawler, **kwargs) self._mockcalled = [] def download(self, request, info): @@ -377,7 +378,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase): def _assert_request_no3xx(self, pipeline_class, settings): - pipe = pipeline_class(settings=Settings(settings)) + pipe = pipeline_class(crawler=get_crawler(None, settings)) request = Request("http://url") pipe._modify_media_request(request) @@ -410,3 +411,115 @@ class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase): self._assert_request_no3xx( UserDefinedPipeline, {"USERDEFINEDPIPELINE_MEDIA_ALLOW_REDIRECTS": True} ) + + +class BuildFromCrawlerTestCase(unittest.TestCase): + def setUp(self): + self.crawler = get_crawler(None, {"FILES_STORE": "/foo"}) + + def test_simple(self): + class Pipeline(UserDefinedPipeline): + pass + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 0) + + def test_has_old_init(self): + class Pipeline(UserDefinedPipeline): + def __init__(self): + super().__init__() + self._init_called = True + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 2) + assert pipe._init_called + + def test_has_from_settings(self): + class Pipeline(UserDefinedPipeline): + _from_settings_called = False + + @classmethod + def from_settings(cls, settings): + o = cls() + o._from_settings_called = True + return o + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 1) + assert pipe._from_settings_called + + def test_has_from_settings_and_init(self): + class Pipeline(UserDefinedPipeline): + _from_settings_called = False + + def __init__(self, store_uri, settings): + super().__init__() + self._init_called = True + + @classmethod + def from_settings(cls, settings): + store_uri = settings["FILES_STORE"] + o = cls(store_uri, settings=settings) + o._from_settings_called = True + return o + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 1) + assert pipe._from_settings_called + assert pipe._init_called + + def test_has_from_crawler_and_init(self): + class Pipeline(UserDefinedPipeline): + _from_crawler_called = False + + def __init__(self, store_uri, settings, *, crawler): + super().__init__(crawler=crawler) + self._init_called = True + + @classmethod + def from_crawler(cls, crawler): + settings = crawler.settings + store_uri = settings["FILES_STORE"] + o = cls(store_uri, settings=settings, crawler=crawler) + o._from_crawler_called = True + return o + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 0) + assert pipe._from_crawler_called + assert pipe._init_called + + def test_has_from_crawler(self): + class Pipeline(UserDefinedPipeline): + _from_crawler_called = False + + @classmethod + def from_crawler(cls, crawler): + settings = crawler.settings + o = super().from_crawler(crawler) + o._from_crawler_called = True + o.store_uri = settings["FILES_STORE"] + return o + + with warnings.catch_warnings(record=True) as w: + pipe = Pipeline.from_crawler(self.crawler) + # this and the next assert will fail as MediaPipeline.from_crawler() wasn't called + assert pipe.crawler == self.crawler + assert pipe._fingerprinter + self.assertEqual(len(w), 0) + assert pipe._from_crawler_called diff --git a/tests/test_spidermiddleware_urllength.py b/tests/test_spidermiddleware_urllength.py index 9111e4c82..1a0f2e223 100644 --- a/tests/test_spidermiddleware_urllength.py +++ b/tests/test_spidermiddleware_urllength.py @@ -3,7 +3,6 @@ from unittest import TestCase from testfixtures import LogCapture from scrapy.http import Request, Response -from scrapy.settings import Settings from scrapy.spidermiddlewares.urllength import UrlLengthMiddleware from scrapy.spiders import Spider from scrapy.utils.test import get_crawler @@ -12,12 +11,10 @@ from scrapy.utils.test import get_crawler class TestUrlLengthMiddleware(TestCase): def setUp(self): self.maxlength = 25 - settings = Settings({"URLLENGTH_LIMIT": self.maxlength}) - - crawler = get_crawler(Spider) + crawler = get_crawler(Spider, {"URLLENGTH_LIMIT": self.maxlength}) self.spider = crawler._create_spider("foo") self.stats = crawler.stats - self.mw = UrlLengthMiddleware.from_settings(settings) + self.mw = UrlLengthMiddleware.from_crawler(crawler) self.response = Response("http://scrapytest.org") self.short_url_req = Request("http://scrapytest.org/") diff --git a/tests/test_utils_misc/__init__.py b/tests/test_utils_misc/__init__.py index 4d8e71521..f71b2b034 100644 --- a/tests/test_utils_misc/__init__.py +++ b/tests/test_utils_misc/__init__.py @@ -10,7 +10,6 @@ from scrapy.item import Field, Item from scrapy.utils.misc import ( arg_to_iter, build_from_crawler, - build_from_settings, create_instance, load_object, rel_has_nofollow, @@ -197,39 +196,6 @@ class UtilsMiscTestCase(unittest.TestCase): with self.assertRaises(TypeError): build_from_crawler(m, crawler, *args, **kwargs) - def test_build_from_settings(self): - settings = mock.MagicMock() - args = (True, 100.0) - kwargs = {"key": "val"} - - def _test_with_settings(mock, settings): - build_from_settings(mock, settings, *args, **kwargs) - if hasattr(mock, "from_settings"): - mock.from_settings.assert_called_once_with(settings, *args, **kwargs) - self.assertEqual(mock.call_count, 0) - else: - mock.assert_called_once_with(*args, **kwargs) - - # Check usage of correct constructor using three mocks: - # 1. with no alternative constructors - # 2. with from_settings() constructor - # 3. with from_settings() and from_crawler() constructor - spec_sets = ( - ["__qualname__"], - ["__qualname__", "from_settings"], - ["__qualname__", "from_settings", "from_crawler"], - ) - for specs in spec_sets: - m = mock.MagicMock(spec_set=specs) - _test_with_settings(m, settings) - m.reset_mock() - - # Check adoption of crawler settings - m = mock.MagicMock(spec_set=["__qualname__", "from_settings"]) - m.from_settings.return_value = None - with self.assertRaises(TypeError): - build_from_settings(m, settings, *args, **kwargs) - def test_set_environ(self): assert os.environ.get("some_test_environ") is None with set_environ(some_test_environ="test_value"): diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index 965d050a4..0a3e3b00b 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -8,6 +8,7 @@ from weakref import WeakKeyDictionary import pytest +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request from scrapy.utils.python import to_bytes from scrapy.utils.request import ( @@ -384,7 +385,9 @@ class CustomRequestFingerprinterTestCase(unittest.TestCase): "REQUEST_FINGERPRINTER_CLASS": RequestFingerprinter, "FINGERPRINT": b"fingerprint", } - crawler = get_crawler(settings_dict=settings) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", ScrapyDeprecationWarning) + crawler = get_crawler(settings_dict=settings) request = Request("http://www.example.com") fingerprint = crawler.request_fingerprinter.fingerprint(request) diff --git a/tests/test_webclient.py b/tests/test_webclient.py index cce119001..1797d5e1f 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -9,25 +9,18 @@ from tempfile import mkdtemp import OpenSSL.SSL from twisted.internet import defer, reactor +from twisted.internet.defer import inlineCallbacks +from twisted.internet.testing import StringTransport +from twisted.protocols.policies import WrappingFactory from twisted.trial import unittest from twisted.web import resource, server, static, util -try: - from twisted.internet.testing import StringTransport -except ImportError: - # deprecated in Twisted 19.7.0 - # (remove once we bump our requirement past that version) - from twisted.test.proto_helpers import StringTransport - -from twisted.internet.defer import inlineCallbacks -from twisted.protocols.policies import WrappingFactory - from scrapy.core.downloader import webclient as client from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory from scrapy.http import Headers, Request -from scrapy.settings import Settings -from scrapy.utils.misc import build_from_settings +from scrapy.utils.misc import build_from_crawler from scrapy.utils.python import to_bytes, to_unicode +from scrapy.utils.test import get_crawler from tests.mockserver import ( BrokenDownloadResource, ErrorResource, @@ -469,22 +462,22 @@ class WebClientCustomCiphersSSLTestCase(WebClientSSLTestCase): def testPayload(self): s = "0123456789" * 10 - settings = Settings({"DOWNLOADER_CLIENT_TLS_CIPHERS": self.custom_ciphers}) - client_context_factory = build_from_settings( - ScrapyClientContextFactory, settings + crawler = get_crawler( + settings_dict={"DOWNLOADER_CLIENT_TLS_CIPHERS": self.custom_ciphers} ) + client_context_factory = build_from_crawler(ScrapyClientContextFactory, crawler) return getPage( self.getURL("payload"), body=s, contextFactory=client_context_factory ).addCallback(self.assertEqual, to_bytes(s)) def testPayloadDisabledCipher(self): s = "0123456789" * 10 - settings = Settings( - {"DOWNLOADER_CLIENT_TLS_CIPHERS": "ECDHE-RSA-AES256-GCM-SHA384"} - ) - client_context_factory = build_from_settings( - ScrapyClientContextFactory, settings + crawler = get_crawler( + settings_dict={ + "DOWNLOADER_CLIENT_TLS_CIPHERS": "ECDHE-RSA-AES256-GCM-SHA384" + } ) + client_context_factory = build_from_crawler(ScrapyClientContextFactory, crawler) d = getPage( self.getURL("payload"), body=s, contextFactory=client_context_factory )