Merge pull request #1989 from pawelmhm/fix-images-pipeline-uppercase-other

[MRG+1] [image_pipeline] bring back uppercase class attributes
This commit is contained in:
Mikhail Korobov 2016-07-13 14:44:00 +00:00 committed by GitHub
commit 79639d0fec
7 changed files with 373 additions and 75 deletions

View File

@ -191,6 +191,12 @@ For the Images Pipeline, set :setting:`IMAGES_URLS_FIELD` and/or
If you need something more complex and want to override the custom pipeline
behaviour, see :ref:`topics-media-pipeline-override`.
If you have multiple image pipelines inheriting from ImagePipeline and you want
to have different settings in different pipelines you can set setting keys
preceded with uppercase name of your pipeline class. E.g. if your pipeline is
called MyPipeline and you want to have custom IMAGES_URLS_FIELD you define
setting MYPIPELINE_IMAGES_URLS_FIELD and your custom settings will be used.
Additional features
===================
@ -214,6 +220,14 @@ specifies the delay in number of days::
The default value for both settings is 90 days.
If you have pipeline that subclasses FilesPipeline and you'd like to have
different setting for it you can set setting keys preceded by uppercase
class name. E.g. given pipeline class called MyPipeline you can set setting key:
MYPIPELINE_FILES_EXPIRES = 180
and pipeline class MyPipeline will have expiration time set to 180.
.. _topics-images-thumbnails:
Thumbnail generation for images

View File

@ -3,7 +3,7 @@ Files Pipeline
See documentation in topics/media-pipeline.rst
"""
import functools
import hashlib
import os
import os.path
@ -214,11 +214,14 @@ class FilesPipeline(MediaPipeline):
"""
MEDIA_NAME = "file"
EXPIRES = 90
STORE_SCHEMES = {
'': FSFilesStore,
'file': FSFilesStore,
's3': S3FilesStore,
}
DEFAULT_FILES_URLS_FIELD = 'file_urls'
DEFAULT_FILES_RESULT_FIELD = 'files'
def __init__(self, store_uri, download_func=None, settings=None):
if not store_uri:
@ -226,11 +229,24 @@ class FilesPipeline(MediaPipeline):
if isinstance(settings, dict) or settings is None:
settings = Settings(settings)
cls_name = "FilesPipeline"
self.store = self._get_store(store_uri)
self.expires = settings.getint('FILES_EXPIRES')
self.files_urls_field = settings.get('FILES_URLS_FIELD')
self.files_result_field = settings.get('FILES_RESULT_FIELD')
resolve = functools.partial(self._key_for_pipe,
base_class_name=cls_name)
self.expires = settings.getint(
resolve('FILES_EXPIRES'), self.EXPIRES
)
if not hasattr(self, "FILES_URLS_FIELD"):
self.FILES_URLS_FIELD = self.DEFAULT_FILES_URLS_FIELD
if not hasattr(self, "FILES_RESULT_FIELD"):
self.FILES_RESULT_FIELD = self.DEFAULT_FILES_RESULT_FIELD
self.files_urls_field = settings.get(
resolve('FILES_URLS_FIELD'), self.FILES_URLS_FIELD
)
self.files_result_field = settings.get(
resolve('FILES_RESULT_FIELD'), self.FILES_RESULT_FIELD
)
super(FilesPipeline, self).__init__(download_func=download_func)

View File

@ -3,7 +3,7 @@ Images Pipeline
See documentation in topics/media-pipeline.rst
"""
import functools
import hashlib
import six
@ -38,25 +38,56 @@ class ImagesPipeline(FilesPipeline):
MEDIA_NAME = 'image'
# Uppercase attributes kept for backward compatibility with code that subclasses
# ImagesPipeline. They may be overridden by settings.
MIN_WIDTH = 0
MIN_HEIGHT = 0
EXPIRES = 0
THUMBS = {}
DEFAULT_IMAGES_URLS_FIELD = 'image_urls'
DEFAULT_IMAGES_RESULT_FIELD = 'images'
def __init__(self, store_uri, download_func=None, settings=None):
super(ImagesPipeline, self).__init__(store_uri, settings=settings, download_func=download_func)
super(ImagesPipeline, self).__init__(store_uri, settings=settings,
download_func=download_func)
if isinstance(settings, dict) or settings is None:
settings = Settings(settings)
self.expires = settings.getint('IMAGES_EXPIRES')
self.images_urls_field = settings.get('IMAGES_URLS_FIELD')
self.images_result_field = settings.get('IMAGES_RESULT_FIELD')
self.min_width = settings.getint('IMAGES_MIN_WIDTH')
self.min_height = settings.getint('IMAGES_MIN_HEIGHT')
self.thumbs = settings.get('IMAGES_THUMBS')
resolve = functools.partial(self._key_for_pipe,
base_class_name="ImagesPipeline")
self.expires = settings.getint(
resolve("IMAGES_EXPIRES"), self.EXPIRES
)
if not hasattr(self, "IMAGES_RESULT_FIELD"):
self.IMAGES_RESULT_FIELD = self.DEFAULT_IMAGES_RESULT_FIELD
if not hasattr(self, "IMAGES_URLS_FIELD"):
self.IMAGES_URLS_FIELD = self.DEFAULT_IMAGES_URLS_FIELD
self.images_urls_field = settings.get(
resolve('IMAGES_URLS_FIELD'),
self.IMAGES_URLS_FIELD
)
self.images_result_field = settings.get(
resolve('IMAGES_RESULT_FIELD'),
self.IMAGES_RESULT_FIELD
)
self.min_width = settings.getint(
resolve('IMAGES_MIN_WIDTH'), self.MIN_WIDTH
)
self.min_height = settings.getint(
resolve('IMAGES_MIN_HEIGHT'), self.MIN_HEIGHT
)
self.thumbs = settings.get(
resolve('IMAGES_THUMBS'), self.THUMBS
)
@classmethod
def from_settings(cls, settings):
s3store = cls.STORE_SCHEMES['s3']
s3store.AWS_ACCESS_KEY_ID = settings['AWS_ACCESS_KEY_ID']
s3store.AWS_SECRET_ACCESS_KEY = settings['AWS_SECRET_ACCESS_KEY']
store_uri = settings['IMAGES_STORE']
return cls(store_uri, settings=settings)

View File

@ -27,6 +27,21 @@ class MediaPipeline(object):
def __init__(self, download_func=None):
self.download_func = download_func
def _key_for_pipe(self, key, base_class_name=None):
"""
>>> MediaPipeline()._key_for_pipe("IMAGES")
'IMAGES'
>>> class MyPipe(MediaPipeline):
... pass
>>> MyPipe()._key_for_pipe("IMAGES", base_class_name="MediaPipeline")
'MYPIPE_IMAGES'
"""
class_name = self.__class__.__name__
if class_name == base_class_name or not base_class_name:
return key
return "{}_{}".format(class_name.upper(), key)
@classmethod
def from_crawler(cls, crawler):
try:

View File

@ -161,9 +161,6 @@ FEED_EXPORTERS_BASE = {
}
FILES_STORE_S3_ACL = 'private'
FILES_EXPIRES = 90
FILES_URLS_FIELD = 'file_urls'
FILES_RESULT_FIELD = 'files'
HTTPCACHE_ENABLED = False
HTTPCACHE_DIR = 'httpcache'
@ -180,13 +177,6 @@ HTTPCACHE_GZIP = False
HTTPPROXY_AUTH_ENCODING = 'latin-1'
IMAGES_MIN_WIDTH = 0
IMAGES_MIN_HEIGHT = 0
IMAGES_EXPIRES = 90
IMAGES_THUMBS = {}
IMAGES_URLS_FIELD = 'image_urls'
IMAGES_RESULT_FIELD = 'images'
ITEM_PROCESSOR = 'scrapy.pipelines.ItemPipelineManager'
ITEM_PIPELINES = {}

View File

@ -1,4 +1,5 @@
import os
import random
import time
import hashlib
import warnings
@ -184,32 +185,140 @@ class FilesPipelineTestCaseFields(unittest.TestCase):
class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
default_cls_settings = {
"EXPIRES": 90,
"FILES_URLS_FIELD": "file_urls",
"FILES_RESULT_FIELD": "files"
}
file_cls_attr_settings_map = {
("EXPIRES", "FILES_EXPIRES", "expires"),
("FILES_URLS_FIELD", "FILES_URLS_FIELD", "files_urls_field"),
("FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field")
}
def setUp(self):
self.tempdir = mkdtemp()
self.pipeline = FilesPipeline(self.tempdir)
self.default_settings = Settings()
def tearDown(self):
rmtree(self.tempdir)
def test_expires(self):
another_pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir,
'FILES_EXPIRES': 42}))
self.assertEqual(self.pipeline.expires, self.default_settings.getint('FILES_EXPIRES'))
self.assertEqual(another_pipeline.expires, 42)
def _generate_fake_settings(self, prefix=None):
def test_files_urls_field(self):
another_pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir,
'FILES_URLS_FIELD': 'funny_field'}))
self.assertEqual(self.pipeline.files_urls_field, self.default_settings.get('FILES_URLS_FIELD'))
self.assertEqual(another_pipeline.files_urls_field, 'funny_field')
def random_string():
return "".join([chr(random.randint(97, 123)) for _ in range(10)])
def test_files_result_field(self):
another_pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir,
'FILES_RESULT_FIELD': 'funny_field'}))
self.assertEqual(self.pipeline.files_result_field, self.default_settings.get('FILES_RESULT_FIELD'))
self.assertEqual(another_pipeline.files_result_field, 'funny_field')
settings = {
"FILES_EXPIRES": random.randint(1, 1000),
"FILES_URLS_FIELD": random_string(),
"FILES_RESULT_FIELD": random_string(),
"FILES_STORE": self.tempdir
}
if not prefix:
return settings
return {prefix.upper() + "_" + k if k != "FILES_STORE" else k: v for k, v in settings.items()}
def _generate_fake_pipeline(self):
class UserDefinedFilePipeline(FilesPipeline):
EXPIRES = 1001
FILES_URLS_FIELD = "alfa"
FILES_RESULT_FIELD = "beta"
return UserDefinedFilePipeline
def test_different_settings_for_different_instances(self):
"""
If there are different instances with different settings they should keep
different settings.
"""
custom_settings = self._generate_fake_settings()
another_pipeline = FilesPipeline.from_settings(Settings(custom_settings))
one_pipeline = FilesPipeline(self.tempdir)
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)
custom_value = custom_settings[settings_attr]
self.assertNotEqual(default_value, custom_value)
self.assertEqual(getattr(another_pipeline, pipe_ins_attr), custom_value)
def test_subclass_attributes_preserved_if_no_settings(self):
"""
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}))
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])
self.assertEqual(getattr(pipe, pipe_ins_attr), getattr(pipe, pipe_attr))
def test_subclass_attrs_preserved_custom_settings(self):
"""
If file settings are defined but they are not defined for subclass class attributes
should be preserved.
"""
pipeline_cls = self._generate_fake_pipeline()
settings = self._generate_fake_settings()
pipeline = pipeline_cls.from_settings(Settings(settings))
for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
value = getattr(pipeline, pipe_ins_attr)
self.assertNotEqual(value, self.default_cls_settings[pipe_attr])
self.assertEqual(value, getattr(pipeline, pipe_attr))
def test_no_custom_settings_for_subclasses(self):
"""
If there are no settings for subclass and no subclass attributes, pipeline should use
attributes of base class.
"""
class UserDefinedFilesPipeline(FilesPipeline):
pass
user_pipeline = UserDefinedFilesPipeline.from_settings(Settings({"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.
custom_value = self.default_cls_settings.get(pipe_attr.upper())
self.assertEqual(getattr(user_pipeline, pipe_ins_attr), custom_value)
def test_custom_settings_for_subclasses(self):
"""
If there are custom settings for subclass and NO class attributes, pipeline should use custom
settings.
"""
class UserDefinedFilesPipeline(FilesPipeline):
pass
prefix = UserDefinedFilesPipeline.__name__.upper()
settings = self._generate_fake_settings(prefix=prefix)
user_pipeline = UserDefinedFilesPipeline.from_settings(Settings(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)
self.assertNotEqual(custom_value, self.default_cls_settings[pipe_attr])
self.assertEqual(getattr(user_pipeline, pipe_inst_attr), custom_value)
def test_custom_settings_and_class_attrs_for_subclasses(self):
"""
If there are custom settings for subclass AND class attributes
setting keys are preferred and override attributes.
"""
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))
for pipe_cls_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map:
custom_value = settings.get(prefix + "_" + settings_attr)
self.assertNotEqual(custom_value, self.default_cls_settings[pipe_cls_attr])
self.assertEqual(getattr(user_pipeline, pipe_inst_attr), custom_value)
def test_cls_attrs_with_DEFAULT_prefix(self):
class UserDefinedFilesPipeline(FilesPipeline):
DEFAULT_FILES_RESULT_FIELD = "this"
DEFAULT_FILES_URLS_FIELD = "that"
pipeline = UserDefinedFilesPipeline.from_settings(Settings({"FILES_STORE": self.tempdir}))
self.assertEqual(pipeline.files_result_field, "this")
self.assertEqual(pipeline.files_urls_field, "that")
class TestS3FilesStore(unittest.TestCase):

View File

@ -1,5 +1,6 @@
import os
import hashlib
import random
import warnings
from tempfile import mkdtemp, TemporaryFile
from shutil import rmtree
@ -206,51 +207,173 @@ class ImagesPipelineTestCaseFields(unittest.TestCase):
class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
img_cls_attribute_names = [
# Pipeline attribute names with corresponding setting names.
("EXPIRES", "IMAGES_EXPIRES"),
("MIN_WIDTH", "IMAGES_MIN_WIDTH"),
("MIN_HEIGHT", "IMAGES_MIN_HEIGHT"),
("IMAGES_URLS_FIELD", "IMAGES_URLS_FIELD"),
("IMAGES_RESULT_FIELD", "IMAGES_RESULT_FIELD"),
("THUMBS", "IMAGES_THUMBS")
]
# This should match what is defined in ImagesPipeline.
default_pipeline_settings = dict(
MIN_WIDTH=0,
MIN_HEIGHT=0,
EXPIRES=0,
THUMBS={},
IMAGES_URLS_FIELD='image_urls',
IMAGES_RESULT_FIELD='images'
)
def setUp(self):
self.tempdir = mkdtemp()
self.pipeline = ImagesPipeline(self.tempdir)
self.default_settings = Settings()
def tearDown(self):
rmtree(self.tempdir)
def test_expires(self):
another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
'IMAGES_EXPIRES': 42}))
self.assertEqual(self.pipeline.expires, self.default_settings.getint('IMAGES_EXPIRES'))
self.assertEqual(another_pipeline.expires, 42)
def _generate_fake_settings(self, prefix=None):
"""
:param prefix: string for setting keys
:return: dictionary of image pipeline settings
"""
def test_images_urls_field(self):
another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
'IMAGES_URLS_FIELD': 'funny_field'}))
self.assertEqual(self.pipeline.images_urls_field, self.default_settings.get('IMAGES_URLS_FIELD'))
self.assertEqual(another_pipeline.images_urls_field, 'funny_field')
def random_string():
return "".join([chr(random.randint(97, 123)) for _ in range(10)])
def test_images_result_field(self):
another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
'IMAGES_RESULT_FIELD': 'funny_field'}))
self.assertEqual(self.pipeline.images_result_field, self.default_settings.get('IMAGES_RESULT_FIELD'))
self.assertEqual(another_pipeline.images_result_field, 'funny_field')
settings = {
"IMAGES_EXPIRES": random.randint(1, 1000),
"IMAGES_STORE": self.tempdir,
"IMAGES_RESULT_FIELD": random_string(),
"IMAGES_URLS_FIELD": random_string(),
"IMAGES_MIN_WIDTH": random.randint(1, 1000),
"IMAGES_MIN_HEIGHT": random.randint(1, 1000),
"IMAGES_THUMBS": {
'small': (random.randint(1, 1000), random.randint(1, 1000)),
'big': (random.randint(1, 1000), random.randint(1, 1000))
}
}
if not prefix:
return settings
def test_min_width(self):
another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
'IMAGES_MIN_WIDTH': 42}))
self.assertEqual(self.pipeline.min_width, self.default_settings.getint('IMAGES_MIN_WIDTH'))
self.assertEqual(another_pipeline.min_width, 42)
return {prefix.upper() + "_" + k if k != "IMAGES_STORE" else k: v for k, v in settings.items()}
def test_min_height(self):
another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
'IMAGES_MIN_HEIGHT': 42}))
self.assertEqual(self.pipeline.min_height, self.default_settings.getint('IMAGES_MIN_HEIGHT'))
self.assertEqual(another_pipeline.min_height, 42)
def _generate_fake_pipeline_subclass(self):
"""
:return: ImagePipeline class will all uppercase attributes set.
"""
class UserDefinedImagePipeline(ImagesPipeline):
# Values should be in different range than fake_settings.
MIN_WIDTH = random.randint(1000, 2000)
MIN_HEIGHT = random.randint(1000, 2000)
THUMBS = {
'small': (random.randint(1000, 2000), random.randint(1000, 2000)),
'big': (random.randint(1000, 2000), random.randint(1000, 2000))
}
EXPIRES = random.randint(1000, 2000)
IMAGES_URLS_FIELD = "field_one"
IMAGES_RESULT_FIELD = "field_two"
def test_thumbs(self):
custom_thumbs = {'small': (50, 50), 'big': (270, 270)}
another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
'IMAGES_THUMBS': custom_thumbs}))
self.assertEqual(self.pipeline.thumbs, self.default_settings.get('IMAGES_THUMBS'))
self.assertEqual(another_pipeline.thumbs, custom_thumbs)
return UserDefinedImagePipeline
def test_different_settings_for_different_instances(self):
"""
If there are two instances of ImagesPipeline class with different settings, they should
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))
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)
self.assertNotEqual(expected_default_value, custom_value)
self.assertEqual(getattr(default_sts_pipe, pipe_attr.lower()), expected_default_value)
self.assertEqual(getattr(user_sts_pipe, pipe_attr.lower()), custom_value)
def test_subclass_attrs_preserved_default_settings(self):
"""
If image settings are not defined at all subclass of ImagePipeline takes values
from class attributes.
"""
pipeline_cls = self._generate_fake_pipeline_subclass()
pipeline = pipeline_cls.from_settings(Settings({"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())
self.assertNotEqual(attr_value, self.default_pipeline_settings[pipe_attr])
self.assertEqual(attr_value, getattr(pipeline, pipe_attr))
def test_subclass_attrs_preserved_custom_settings(self):
"""
If image settings are defined but they are not defined for subclass class attributes
should be preserved.
"""
pipeline_cls = self._generate_fake_pipeline_subclass()
settings = self._generate_fake_settings()
pipeline = pipeline_cls.from_settings(Settings(settings))
for pipe_attr, settings_attr in self.img_cls_attribute_names:
# Instance attribute (lowercase) must be equal to class attribute (uppercase).
value = getattr(pipeline, pipe_attr.lower())
self.assertNotEqual(value, self.default_pipeline_settings[pipe_attr])
self.assertEqual(value, getattr(pipeline, pipe_attr))
def test_no_custom_settings_for_subclasses(self):
"""
If there are no settings for subclass and no subclass attributes, pipeline should use
attributes of base class.
"""
class UserDefinedImagePipeline(ImagesPipeline):
pass
user_pipeline = UserDefinedImagePipeline.from_settings(Settings({"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.
custom_value = self.default_pipeline_settings.get(pipe_attr.upper())
self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
def test_custom_settings_for_subclasses(self):
"""
If there are custom settings for subclass and NO class attributes, pipeline should use custom
settings.
"""
class UserDefinedImagePipeline(ImagesPipeline):
pass
prefix = UserDefinedImagePipeline.__name__.upper()
settings = self._generate_fake_settings(prefix=prefix)
user_pipeline = UserDefinedImagePipeline.from_settings(Settings(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)
self.assertNotEqual(custom_value, self.default_pipeline_settings[pipe_attr])
self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
def test_custom_settings_and_class_attrs_for_subclasses(self):
"""
If there are custom settings for subclass AND class attributes
setting keys are preferred and override attributes.
"""
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))
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])
self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
def test_cls_attrs_with_DEFAULT_prefix(self):
class UserDefinedImagePipeline(ImagesPipeline):
DEFAULT_IMAGES_URLS_FIELD = "something"
DEFAULT_IMAGES_RESULT_FIELD = "something_else"
pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
self.assertEqual(pipeline.images_result_field, "something_else")
self.assertEqual(pipeline.images_urls_field, "something")
def _create_image(format, *a, **kw):