Modernize the media pipeline initialization API.

This commit is contained in:
Andrey Rakhmatullin 2024-11-14 01:29:51 +05:00
parent 8700a5b7a9
commit 28fafbb8c5
6 changed files with 211 additions and 80 deletions

View File

@ -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.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, 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__)
@ -443,6 +446,8 @@ 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:
@ -467,10 +472,35 @@ 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, 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"{cls.__name__} 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:
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"]
@ -495,7 +525,18 @@ class FilesPipeline(MediaPipeline):
ftp_store.USE_ACTIVE_MODE = settings.getbool("FEED_STORAGE_FTP_ACTIVE")
store_uri = settings["FILES_STORE"]
return cls(store_uri, settings=settings)
if "crawler" in get_func_args(cls.__init__):
o = cls(store_uri, settings=settings, crawler=crawler)
else:
o = cls(store_uri, settings=settings)
if crawler:
o._finish_init(crawler)
warnings.warn(
f"{cls.__qualname__}.__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 _get_store(self, uri: str) -> FilesStoreProtocol:
if Path(uri).is_absolute(): # to support win32 paths like: C:\\some\dir

View File

@ -8,13 +8,14 @@ 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 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 (
@ -26,7 +27,7 @@ from scrapy.pipelines.files import (
_md5sum,
)
from scrapy.settings import Settings
from scrapy.utils.python import to_bytes
from scrapy.utils.python import get_func_args, to_bytes
if TYPE_CHECKING:
from collections.abc import Callable, Iterable
@ -38,6 +39,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 +66,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,7 +78,9 @@ 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, download_func=download_func, crawler=crawler
)
if isinstance(settings, dict) or settings is None:
settings = Settings(settings)
@ -108,7 +114,7 @@ class ImagesPipeline(FilesPipeline):
)
@classmethod
def from_settings(cls, settings: Settings) -> Self:
def _from_settings(cls, settings: Settings, crawler: Crawler | None) -> 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"]
@ -133,7 +139,18 @@ class ImagesPipeline(FilesPipeline):
ftp_store.USE_ACTIVE_MODE = settings.getbool("FEED_STORAGE_FTP_ACTIVE")
store_uri = settings["IMAGES_STORE"]
return cls(store_uri, settings=settings)
if "crawler" in get_func_args(cls.__init__):
o = cls(store_uri, settings=settings, crawler=crawler)
else:
o = cls(store_uri, settings=settings)
if crawler:
o._finish_init(crawler)
warnings.warn(
f"{cls.__qualname__}.__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,

View File

@ -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
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,6 +76,8 @@ 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
@ -87,6 +91,28 @@ class MediaPipeline(ABC):
)
self._handle_statuses(self.allow_redirects)
if crawler:
# TODO use crawler.settings
self._finish_init(crawler)
self._modern_init = True
else:
warnings.warn(
f"MediaPipeline.__init__() was called without the crawler argument"
f" when creating {self.__class__.__qualname__}."
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 +138,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"{cls.__qualname__}.__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:

View File

@ -17,7 +17,6 @@ from itemadapter import ItemAdapter
from twisted.internet import defer
from twisted.trial import unittest
from scrapy import Spider
from scrapy.http import Request, Response
from scrapy.item import Field, Item
from scrapy.pipelines.files import (
@ -27,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,
@ -219,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")
@ -237,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})]
@ -249,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))
@ -373,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)
@ -387,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])
@ -400,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)
@ -416,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.
@ -435,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)
@ -450,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,
@ -465,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,
@ -486,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)
@ -497,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"))
@ -695,7 +700,7 @@ def _prepare_request_object(item_url, flags=None):
class BuildFromCrawlerTestCase(unittest.TestCase):
def setUp(self):
self.tempdir = mkdtemp()
self.crawler = get_crawler(Spider, {"FILES_STORE": self.tempdir})
self.crawler = get_crawler(None, {"FILES_STORE": self.tempdir})
def tearDown(self):
rmtree(self.tempdir)
@ -711,8 +716,23 @@ class BuildFromCrawlerTestCase(unittest.TestCase):
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)
@ -723,27 +743,24 @@ class BuildFromCrawlerTestCase(unittest.TestCase):
pipe = Pipeline.from_crawler(self.crawler)
assert pipe.crawler == self.crawler
assert pipe._fingerprinter
self.assertEqual(len(w), 0)
self.assertEqual(len(w), 3)
assert pipe.store
assert pipe._from_settings_called
@pytest.mark.xfail(
reason="No way to override MediaPipeline.from_crawler having non-trivial __init__"
)
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"]
# you can either call super().from_crawler() or cls.__init__() but you need both
o = cls(store_uri, settings=settings)
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)
# 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)

View File

@ -14,6 +14,7 @@ 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 +34,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 +125,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 +220,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 +234,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))
@ -390,8 +393,10 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
"""
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, settings=default_settings, crawler=get_crawler(None) # TODO
)
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 +412,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 +428,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 +446,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 +465,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 +482,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 +493,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 +515,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)

View File

@ -2,7 +2,6 @@ from __future__ import annotations
import warnings
import pytest
from testfixtures import LogCapture
from twisted.internet import reactor
from twisted.internet.defer import Deferred, inlineCallbacks
@ -14,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
@ -178,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):
@ -380,7 +378,8 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase):
class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase):
def _assert_request_no3xx(self, pipeline_class, settings):
pipe = pipeline_class(settings=Settings(settings))
crawler = get_crawler(None, settings)
pipe = pipeline_class(settings=settings, crawler=crawler) # TODO
request = Request("http://url")
pipe._modify_media_request(request)
@ -417,7 +416,7 @@ class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase):
class BuildFromCrawlerTestCase(unittest.TestCase):
def setUp(self):
self.crawler = get_crawler(Spider, {"FILES_STORE": "/foo"})
self.crawler = get_crawler(None, {"FILES_STORE": "/foo"})
def test_simple(self):
class Pipeline(UserDefinedPipeline):
@ -429,8 +428,23 @@ class BuildFromCrawlerTestCase(unittest.TestCase):
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()
@ -441,11 +455,13 @@ class BuildFromCrawlerTestCase(unittest.TestCase):
pipe = Pipeline.from_crawler(self.crawler)
assert pipe.crawler == self.crawler
assert pipe._fingerprinter
self.assertEqual(len(w), 0)
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
@ -461,31 +477,28 @@ class BuildFromCrawlerTestCase(unittest.TestCase):
pipe = Pipeline.from_crawler(self.crawler)
assert pipe.crawler == self.crawler
assert pipe._fingerprinter
self.assertEqual(len(w), 0)
self.assertEqual(len(w), 1)
assert pipe._from_settings_called
assert pipe._init_called
@pytest.mark.xfail(
reason="No way to override MediaPipeline.from_crawler having non-trivial __init__"
)
def test_has_from_crawler_and_init(self):
class Pipeline(UserDefinedPipeline):
def __init__(self, store_uri, settings):
super().__init__()
_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"]
# you can either call super().from_crawler() or cls.__init__() but you need both
o = cls(store_uri, settings=settings)
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)
# this and the next assert will fail as super().from_crawler() wasn't called
assert pipe.crawler == self.crawler
assert pipe._fingerprinter
self.assertEqual(len(w), 0)
@ -494,6 +507,8 @@ class BuildFromCrawlerTestCase(unittest.TestCase):
def test_has_from_crawler(self):
class Pipeline(UserDefinedPipeline):
_from_crawler_called = False
@classmethod
def from_crawler(cls, crawler):
settings = crawler.settings