From a769a1ef784a4383bc2f740d3a74b1e6cc6aeff9 Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Fri, 19 Jun 2015 15:01:24 +0200 Subject: [PATCH 1/8] Introduce BaseSettings with full dictionary interface --- docs/topics/api.rst | 75 ++++++++--- scrapy/settings/__init__.py | 89 +++++++++++-- tests/test_cmdline/__init__.py | 14 +++ tests/test_cmdline/extensions.py | 5 + tests/test_settings/__init__.py | 159 ++++++++++++++++++------ tests/test_settings/default_settings.py | 3 + 6 files changed, 280 insertions(+), 65 deletions(-) diff --git a/docs/topics/api.rst b/docs/topics/api.rst index f54341eb8..923bd80b0 100644 --- a/docs/topics/api.rst +++ b/docs/topics/api.rst @@ -140,26 +140,41 @@ Settings API For a detailed explanation on each settings sources, see: :ref:`topics-settings`. +.. function:: get_settings_priority(priority) + + Small helper function that looks up a given string priority in the + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` dictionary and returns its + numerical value, or directly returns a given numerical priority. + .. class:: Settings(values={}, priority='project') This object stores Scrapy settings for the configuration of internal components, and can be used for any further customization. - After instantiation of this class, the new object will have the global - default settings described on :ref:`topics-settings-ref` already - populated. + It is a direct subclass and supports all methods of + :class:`~scrapy.settings.BaseSettings`. Additionally, after instantiation + of this class, the new object will have the global default settings + described on :ref:`topics-settings-ref` already populated. - Additional values can be passed on initialization with the ``values`` - argument, and they would take the ``priority`` level. If the latter +.. class:: BaseSettings(values={}, priority='project') + + Instances of this class behave like dictionaries, but store priorities + along with their ``(key, value)`` pairs, and can be frozen (i.e. marked + immutable). + + Key-value entries can be passed on initialization with the ``values`` + argument, and they would take the ``priority`` level (unless ``values`` is + already an instance of :class:`~scrapy.settings.BaseSettings`, in which + case the existing priority levels will be kept). If the ``priority`` argument is a string, the priority name will be looked up in - :attr:`~scrapy.settings.SETTINGS_PRIORITIES`. Otherwise, a expecific - integer should be provided. + :attr:`~scrapy.settings.SETTINGS_PRIORITIES`. Otherwise, a specific integer + should be provided. Once the object is created, new settings can be loaded or updated with the - :meth:`~scrapy.settings.Settings.set` method, and can be accessed with the - square bracket notation of dictionaries, or with the - :meth:`~scrapy.settings.Settings.get` method of the instance and its value - conversion variants. When requesting a stored key, the value with the + :meth:`~scrapy.settings.BaseSettings.set` method, and can be accessed with + the square bracket notation of dictionaries, or with the + :meth:`~scrapy.settings.BaseSettings.get` method of the instance and its + value conversion variants. When requesting a stored key, the value with the highest priority will be retrieved. .. method:: set(name, value, priority='project') @@ -180,16 +195,23 @@ Settings API :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer :type priority: string or int - .. method:: setdict(values, priority='project') + .. method:: update(values, priority='project') Store key/value pairs with a given priority. This is a helper function that calls - :meth:`~scrapy.settings.Settings.set` for every item of ``values`` + :meth:`~scrapy.settings.BaseSettings.set` for every item of ``values`` with the provided ``priority``. + If ``values`` is a string, it is assumed to be JSON-encoded and parsed + into a dict with ``json.loads()`` first. If it is a + :class:`~scrapy.settings.BaseSettings` instance, the per-key priorities + will be used and the ``priority`` parameter ignored. This allows + inserting/updating settings with different priorities with a single + command. + :param values: the settings names and values - :type values: dict + :type values: dict or string or :class:`~scrapy.settings.BaseSettings` :param priority: the priority of the settings. Should be a key of :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer @@ -200,7 +222,7 @@ Settings API Store settings from a module with a given priority. This is a helper function that calls - :meth:`~scrapy.settings.Settings.set` for every globally declared + :meth:`~scrapy.settings.BaseSettings.set` for every globally declared uppercase variable of ``module`` with the provided ``priority``. :param module: the module or the path of the module @@ -272,8 +294,12 @@ Settings API .. method:: getdict(name, default=None) Get a setting value as a dictionary. If the setting original type is a - dictionary, a copy of it will be returned. If it's a string it will - evaluated as a json dictionary. + dictionary, a copy of it will be returned. If it is a string it will be + evaluated as a JSON dictionary. In the case that it is a + :class:`~scrapy.settings.BaseSettings` instance itself, it will be + converted to a dictionary, containing all its current settings values + as they would be returned by :meth:`~scrapy.settings.BaseSettings.get`, + and losing all information about priority and mutability. :param name: the setting name :type name: string @@ -305,6 +331,21 @@ Settings API Alias for a :meth:`~freeze` call in the object returned by :meth:`copy` + .. method:: getpriority(name) + + Return the current numerical priority value of a setting, or ``None`` if + the given ``name`` does not exist. + + :param name: the setting name + :type name: string + + .. method:: maxpriority() + + Return the numerical value of the highest priority present throughout + all settings, or the numerical value for ``default`` from + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` if there are no settings + stored. + .. _topics-api-spiderloader: SpiderLoader API diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index af0d0dff1..fa7fa3178 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -2,7 +2,7 @@ import six import json import copy import warnings -from collections import MutableMapping +from collections import Mapping, MutableMapping from importlib import import_module from scrapy.utils.deprecate import create_deprecated_class @@ -19,6 +19,12 @@ SETTINGS_PRIORITIES = { 'cmdline': 40, } +def get_settings_priority(priority): + if isinstance(priority, six.string_types): + return SETTINGS_PRIORITIES[priority] + else: + return priority + class SettingsAttribute(object): @@ -45,21 +51,22 @@ class SettingsAttribute(object): __repr__ = __str__ -class Settings(object): +class BaseSettings(MutableMapping): def __init__(self, values=None, priority='project'): self.frozen = False self.attributes = {} - self.setmodule(default_settings, priority='default') - if values is not None: - self.setdict(values, priority) + self.update(values, priority) def __getitem__(self, opt_name): value = None - if opt_name in self.attributes: + if opt_name in self: value = self.attributes[opt_name].value return value + def __contains__(self, name): + return name in self.attributes + def get(self, name, default=None): return self[name] if self[name] is not None else default @@ -88,19 +95,34 @@ class Settings(object): value = json.loads(value) return dict(value) + def getpriority(self, name): + prio = None + if name in self: + prio = self.attributes[name].priority + return prio + + def maxpriority(self): + if len(self) > 0: + return max(self.getpriority(name) for name in self) + else: + return get_settings_priority('default') + + def __setitem__(self, name, value): + self.set(name, value) + def set(self, name, value, priority='project'): self._assert_mutability() - if isinstance(priority, six.string_types): - priority = SETTINGS_PRIORITIES[priority] - if name not in self.attributes: - self.attributes[name] = SettingsAttribute(value, priority) + priority = get_settings_priority(priority) + if name not in self: + if isinstance(value, SettingsAttribute): + self.attributes[name] = value + else: + self.attributes[name] = SettingsAttribute(value, priority) else: self.attributes[name].set(value, priority) def setdict(self, values, priority='project'): - self._assert_mutability() - for name, value in six.iteritems(values): - self.set(name, value, priority) + self.update(values, priority) def setmodule(self, module, priority='project'): self._assert_mutability() @@ -110,6 +132,28 @@ class Settings(object): if key.isupper(): self.set(key, getattr(module, key), priority) + def update(self, values, priority='project'): + self._assert_mutability() + if isinstance(values, six.string_types): + values = json.loads(values) + if values is not None: + if isinstance(values, BaseSettings): + for name, value in six.iteritems(values): + self.set(name, value, values.getpriority(name)) + else: + for name, value in six.iteritems(values): + self.set(name, value, priority) + + def delete(self, name, priority='project'): + self._assert_mutability() + priority = get_settings_priority(priority) + if priority >= self.getpriority(name): + del self.attributes[name] + + def __delitem__(self, name): + self._assert_mutability() + del self.attributes[name] + def _assert_mutability(self): if self.frozen: raise TypeError("Trying to modify an immutable Settings object") @@ -125,6 +169,17 @@ class Settings(object): copy.freeze() return copy + def __iter__(self): + return iter(self.attributes) + + def __len__(self): + return len(self.attributes) + + def __str__(self): + return str(self.attributes) + + __repr__ = __str__ + @property def overrides(self): warnings.warn("`Settings.overrides` attribute is deprecated and won't " @@ -174,6 +229,14 @@ class _DictProxy(MutableMapping): return iter(self.o) +class Settings(BaseSettings): + + def __init__(self, values=None, priority='project'): + super(Settings, self).__init__() + self.setmodule(default_settings, 'default') + self.update(values, priority) + + class CrawlerSettings(Settings): def __init__(self, settings_module=None, **kw): diff --git a/tests/test_cmdline/__init__.py b/tests/test_cmdline/__init__.py index 1e2905e95..5192fb0fa 100644 --- a/tests/test_cmdline/__init__.py +++ b/tests/test_cmdline/__init__.py @@ -1,4 +1,5 @@ import os +import json import sys import shutil import pstats @@ -54,3 +55,16 @@ class CmdlineTest(unittest.TestCase): self.assertIn('tottime', stats) finally: shutil.rmtree(path) + + def test_override_dict_settings(self): + settingsstr = self._execute('settings', '--get', 'EXTENSIONS', '-s', + ('EXTENSIONS={"tests.test_cmdline.extensions.TestExtension": ' + '100, "tests.test_cmdline.extensions.DummyExtension": 200}')) + # XXX: There's gotta be a smarter way to do this... + self.assertNotIn("...", settingsstr) + for char in ("'", "<", ">", 'u"'): + settingsstr = settingsstr.replace(char, '"') + settingsdict = json.loads(settingsstr) + self.assertIn('tests.test_cmdline.extensions.DummyExtension', settingsdict) + self.assertIn('value=200', settingsdict['tests.test_cmdline.extensions.DummyExtension']) + self.assertIn('value=100', settingsdict['tests.test_cmdline.extensions.TestExtension']) diff --git a/tests/test_cmdline/extensions.py b/tests/test_cmdline/extensions.py index 4d347966a..72867eb56 100644 --- a/tests/test_cmdline/extensions.py +++ b/tests/test_cmdline/extensions.py @@ -8,3 +8,8 @@ class TestExtension(object): @classmethod def from_crawler(cls, crawler): return cls(crawler.settings) + + +class DummyExtension(object): + pass + diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index 54b834aa0..a473f3c3f 100644 --- a/tests/test_settings/__init__.py +++ b/tests/test_settings/__init__.py @@ -2,7 +2,8 @@ import six import unittest import warnings -from scrapy.settings import Settings, SettingsAttribute, CrawlerSettings +from scrapy.settings import (BaseSettings, Settings, SettingsAttribute, + CrawlerSettings) from tests import mock from . import default_settings @@ -33,35 +34,16 @@ class SettingsTest(unittest.TestCase): if six.PY3: assertItemsEqual = unittest.TestCase.assertCountEqual + +class BaseSettingsTest(unittest.TestCase): + + if six.PY3: + assertItemsEqual = unittest.TestCase.assertCountEqual + def setUp(self): - self.settings = Settings() - - @mock.patch.dict('scrapy.settings.SETTINGS_PRIORITIES', {'default': 10}) - @mock.patch('scrapy.settings.default_settings', default_settings) - def test_initial_defaults(self): - settings = Settings() - self.assertEqual(len(settings.attributes), 1) - self.assertIn('TEST_DEFAULT', settings.attributes) - - attr = settings.attributes['TEST_DEFAULT'] - self.assertIsInstance(attr, SettingsAttribute) - self.assertEqual(attr.value, 'defvalue') - self.assertEqual(attr.priority, 10) - - @mock.patch.dict('scrapy.settings.SETTINGS_PRIORITIES', {}) - @mock.patch('scrapy.settings.default_settings', {}) - def test_initial_values(self): - settings = Settings({'TEST_OPTION': 'value'}, 10) - self.assertEqual(len(settings.attributes), 1) - self.assertIn('TEST_OPTION', settings.attributes) - - attr = settings.attributes['TEST_OPTION'] - self.assertIsInstance(attr, SettingsAttribute) - self.assertEqual(attr.value, 'value') - self.assertEqual(attr.priority, 10) + self.settings = BaseSettings() def test_set_new_attribute(self): - self.settings.attributes = {} self.settings.set('TEST_OPTION', 'value', 0) self.assertIn('TEST_OPTION', self.settings.attributes) @@ -70,6 +52,12 @@ class SettingsTest(unittest.TestCase): self.assertEqual(attr.value, 'value') self.assertEqual(attr.priority, 0) + def test_set_settingsattribute(self): + myattr = SettingsAttribute(0, 30) # Note priority 30 + self.settings.set('TEST_ATTR', myattr, 10) + self.assertEqual(self.settings.get('TEST_ATTR'), 0) + self.assertEqual(self.settings.getpriority('TEST_ATTR'), 30) + def test_set_instance_identity_on_update(self): attr = SettingsAttribute('value', 0) self.settings.attributes = {'TEST_OPTION': attr} @@ -79,13 +67,11 @@ class SettingsTest(unittest.TestCase): self.assertIs(attr, self.settings.attributes['TEST_OPTION']) def test_set_calls_settings_attributes_methods_on_update(self): - with mock.patch.object(SettingsAttribute, '__setattr__') as mock_setattr, \ - mock.patch.object(SettingsAttribute, 'set') as mock_set: + attr = SettingsAttribute('value', 10) + with mock.patch.object(attr, '__setattr__') as mock_setattr, \ + mock.patch.object(attr, 'set') as mock_set: - attr = SettingsAttribute('value', 10) self.settings.attributes = {'TEST_OPTION': attr} - mock_set.reset_mock() - mock_setattr.reset_mock() for priority in (0, 10, 20): self.settings.set('TEST_OPTION', 'othervalue', priority) @@ -94,6 +80,19 @@ class SettingsTest(unittest.TestCase): mock_set.reset_mock() mock_setattr.reset_mock() + def test_setitem(self): + settings = BaseSettings() + settings.set('key', 'a', 'default') + settings['key'] = 'b' + self.assertEqual(settings['key'], 'b') + self.assertEqual(settings.getpriority('key'), 20) + settings['key'] = 'c' + self.assertEqual(settings['key'], 'c') + settings['key2'] = 'x' + self.assertIn('key2', settings) + self.assertEqual(settings['key2'], 'x') + self.assertEqual(settings.getpriority('key2'), 20) + def test_setdict_alias(self): with mock.patch.object(self.settings, 'set') as mock_set: self.settings.setdict({'TEST_1': 'value1', 'TEST_2': 'value2'}, 10) @@ -118,7 +117,8 @@ class SettingsTest(unittest.TestCase): def test_setmodule_alias(self): with mock.patch.object(self.settings, 'set') as mock_set: self.settings.setmodule(default_settings, 10) - mock_set.assert_called_with('TEST_DEFAULT', 'defvalue', 10) + mock_set.assert_any_call('TEST_DEFAULT', 'defvalue', 10) + mock_set.assert_any_call('TEST_DICT', {'key': 'val'}, 10) def test_setmodule_by_path(self): self.settings.attributes = {} @@ -132,11 +132,55 @@ class SettingsTest(unittest.TestCase): self.assertItemsEqual(six.iterkeys(self.settings.attributes), six.iterkeys(ctrl_attributes)) - for attr, ctrl_attr in zip(six.itervalues(self.settings.attributes), - six.itervalues(ctrl_attributes)): + for key in six.iterkeys(ctrl_attributes): + attr = self.settings.attributes[key] + ctrl_attr = ctrl_attributes[key] self.assertEqual(attr.value, ctrl_attr.value) self.assertEqual(attr.priority, ctrl_attr.priority) + def test_update(self): + settings = BaseSettings({'key_lowprio': 0}, priority=0) + settings.set('key_highprio', 10, priority=50) + custom_settings = BaseSettings({'key_lowprio': 1, 'key_highprio': 11}, priority=30) + custom_settings.set('newkey_one', None, priority=50) + custom_dict = {'key_lowprio': 2, 'key_highprio': 12, 'newkey_two': None} + + settings.update(custom_dict, priority=20) + self.assertEqual(settings['key_lowprio'], 2) + self.assertEqual(settings.getpriority('key_lowprio'), 20) + self.assertEqual(settings['key_highprio'], 10) + self.assertIn('newkey_two', settings) + self.assertEqual(settings.getpriority('newkey_two'), 20) + + settings.update(custom_settings) + self.assertEqual(settings['key_lowprio'], 1) + self.assertEqual(settings.getpriority('key_lowprio'), 30) + self.assertEqual(settings['key_highprio'], 10) + self.assertIn('newkey_one', settings) + self.assertEqual(settings.getpriority('newkey_one'), 50) + + settings.update({'key_lowprio': 3}, priority=20) + self.assertEqual(settings['key_lowprio'], 1) + + def test_update_jsonstring(self): + settings = BaseSettings({'number': 0, 'dict': BaseSettings({'key': 'val'})}) + settings.update('{"number": 1, "newnumber": 2}') + self.assertEqual(settings['number'], 1) + self.assertEqual(settings['newnumber'], 2) + settings.set("dict", '{"key": "newval", "newkey": "newval2"}') + self.assertEqual(settings['dict']['key'], "newval") + self.assertEqual(settings['dict']['newkey'], "newval2") + + def test_delete(self): + settings = BaseSettings({'key': None}) + settings.set('key_highprio', None, priority=50) + settings.delete('key') + settings.delete('key_highprio') + self.assertNotIn('key', settings) + self.assertIn('key_highprio', settings) + del settings['key_highprio'] + self.assertNotIn('key_highprio', settings) + def test_get(self): test_configuration = { 'TEST_ENABLED1': '1', @@ -190,6 +234,18 @@ class SettingsTest(unittest.TestCase): self.assertEqual(settings.getdict('TEST_DICT3', {'key1': 5}), {'key1': 5}) self.assertRaises(ValueError, settings.getdict, 'TEST_LIST1') + def test_getpriority(self): + settings = BaseSettings({'key': 'value'}, priority=99) + self.assertEqual(settings.getpriority('key'), 99) + self.assertEqual(settings.getpriority('nonexistentkey'), None) + + def test_maxpriority(self): + # Empty settings should return 'default' + self.assertEqual(self.settings.maxpriority(), 0) + self.settings.set('A', 0, 10) + self.settings.set('B', 0, 30) + self.assertEqual(self.settings.maxpriority(), 30) + def test_copy(self): values = { 'TEST_BOOL': True, @@ -254,6 +310,39 @@ class SettingsTest(unittest.TestCase): self.assertIn('BAR', self.settings.defaults) +class SettingsTest(unittest.TestCase): + + if six.PY3: + assertItemsEqual = unittest.TestCase.assertCountEqual + + def setUp(self): + self.settings = Settings() + + @mock.patch.dict('scrapy.settings.SETTINGS_PRIORITIES', {'default': 10}) + @mock.patch('scrapy.settings.default_settings', default_settings) + def test_initial_defaults(self): + settings = Settings() + self.assertEqual(len(settings.attributes), 2) + self.assertIn('TEST_DEFAULT', settings.attributes) + + attr = settings.attributes['TEST_DEFAULT'] + self.assertIsInstance(attr, SettingsAttribute) + self.assertEqual(attr.value, 'defvalue') + self.assertEqual(attr.priority, 10) + + @mock.patch.dict('scrapy.settings.SETTINGS_PRIORITIES', {}) + @mock.patch('scrapy.settings.default_settings', {}) + def test_initial_values(self): + settings = Settings({'TEST_OPTION': 'value'}, 10) + self.assertEqual(len(settings.attributes), 1) + self.assertIn('TEST_OPTION', settings.attributes) + + attr = settings.attributes['TEST_OPTION'] + self.assertIsInstance(attr, SettingsAttribute) + self.assertEqual(attr.value, 'value') + self.assertEqual(attr.priority, 10) + + class CrawlerSettingsTest(unittest.TestCase): def test_deprecated_crawlersettings(self): diff --git a/tests/test_settings/default_settings.py b/tests/test_settings/default_settings.py index 23005d4c6..c24b5a9b9 100644 --- a/tests/test_settings/default_settings.py +++ b/tests/test_settings/default_settings.py @@ -1,2 +1,5 @@ TEST_DEFAULT = 'defvalue' + +TEST_DICT = {'key': 'val'} + From 26586ef5a605c6f6a23143b22812779857cd4e3e Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Fri, 19 Jun 2015 15:09:36 +0200 Subject: [PATCH 2/8] Deprecate _BASE settings, unify _BASE backwards-compatibility --- docs/topics/downloader-middleware.rst | 22 ++- docs/topics/extensions.rst | 20 ++- docs/topics/feed-exports.rst | 41 +++-- docs/topics/settings.rst | 142 +++++++----------- docs/topics/spider-middleware.rst | 22 ++- scrapy/commands/check.py | 5 +- scrapy/commands/crawl.py | 8 +- scrapy/commands/runspider.py | 8 +- scrapy/core/downloader/handlers/__init__.py | 8 +- scrapy/core/downloader/middleware.py | 3 +- scrapy/core/spidermw.py | 3 +- .../downloadermiddlewares/defaultheaders.py | 5 +- scrapy/extension.py | 3 +- scrapy/extensions/feedexport.py | 4 +- scrapy/pipelines/__init__.py | 10 +- scrapy/settings/__init__.py | 41 ++++- scrapy/settings/default_settings.py | 25 +-- scrapy/utils/conf.py | 43 ++++-- tests/test_settings/__init__.py | 49 +++++- tests/test_utils_conf.py | 43 ++++-- 20 files changed, 267 insertions(+), 238 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 9122e5cb5..08d8f3edf 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -23,22 +23,20 @@ Here's an example:: 'myproject.middlewares.CustomDownloaderMiddleware': 543, } -The :setting:`DOWNLOADER_MIDDLEWARES` setting is merged with the -:setting:`DOWNLOADER_MIDDLEWARES_BASE` setting defined in Scrapy (and not meant to -be overridden) and then sorted by order to get the final sorted list of enabled -middlewares: the first middleware is the one closer to the engine and the last -is the one closer to the downloader. +The specified :setting:`DOWNLOADER_MIDDLEWARES` setting is merged with the +default one (i.e. it does not overwrite it) and then sorted by order to get the +final sorted list of enabled middlewares: the first middleware is the one +closer to the engine and the last is the one closer to the downloader. -To decide which order to assign to your middleware see the -:setting:`DOWNLOADER_MIDDLEWARES_BASE` setting and pick a value according to +To decide which order to assign to your middleware see the default +:setting:`DOWNLOADER_MIDDLEWARES` setting and pick a value according to where you want to insert the middleware. The order does matter because each middleware performs a different action and your middleware could depend on some previous (or subsequent) middleware being applied. -If you want to disable a built-in middleware (the ones defined in -:setting:`DOWNLOADER_MIDDLEWARES_BASE` and enabled by default) you must define it -in your project's :setting:`DOWNLOADER_MIDDLEWARES` setting and assign `None` -as its value. For example, if you want to disable the user-agent middleware:: +If you want to disable a built-in middleware you must define it in your +project's :setting:`DOWNLOADER_MIDDLEWARES` setting and assign ``None`` as its +value. For example, if you want to disable the user-agent middleware:: DOWNLOADER_MIDDLEWARES = { 'myproject.middlewares.CustomDownloaderMiddleware': 543, @@ -164,7 +162,7 @@ middleware, see the :ref:`downloader middleware usage guide `. For a list of the components enabled by default (and their orders) see the -:setting:`DOWNLOADER_MIDDLEWARES_BASE` setting. +:setting:`DOWNLOADER_MIDDLEWARES` setting. .. _cookies-mw: diff --git a/docs/topics/extensions.rst b/docs/topics/extensions.rst index fb5220e9d..a71b8bcee 100644 --- a/docs/topics/extensions.rst +++ b/docs/topics/extensions.rst @@ -42,17 +42,15 @@ by a string: the full Python path to the extension's class name. For example:: As you can see, the :setting:`EXTENSIONS` setting is a dict where the keys are the extension paths, and their values are the orders, which define the -extension *loading* order. Extensions orders are not as important as middleware -orders though, and they are typically irrelevant, ie. it doesn't matter in -which order the extensions are loaded because they don't depend on each other -[1]. +extension *loading* order. The specified :setting:`EXTENSIONS` setting is merged +with the default one (i.e. it does not overwrite it) and then sorted by order +to get the final sorted list of enabled extensions. -However, this feature can be exploited if you need to add an extension which -depends on other extensions already loaded. - -[1] This is is why the :setting:`EXTENSIONS_BASE` setting in Scrapy (which -contains all built-in extensions enabled by default) defines all the extensions -with the same order (``500``). +As extensions typically do not depend on each other, their loading order is +irrelevant in most cases. This is why the default :setting:`EXTENSIONS` setting +defines all extensions with the same order (``500``). However, this feature can +be exploited if you need to add an extension which depends on other extensions +already loaded. Available, enabled and disabled extensions ========================================== @@ -65,7 +63,7 @@ Disabling an extension ====================== In order to disable an extension that comes enabled by default (ie. those -included in the :setting:`EXTENSIONS_BASE` setting) you must set its order to +included in the default :setting:`EXTENSIONS` setting) you must set its order to ``None``. For example:: EXTENSIONS = { diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index d9444e34a..d8b8da166 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -265,16 +265,6 @@ Whether to export empty feeds (ie. feeds with no items). FEED_STORAGES ------------- -Default:: ``{}`` - -A dict containing additional feed storage backends supported by your project. -The keys are URI schemes and the values are paths to storage classes. - -.. setting:: FEED_STORAGES_BASE - -FEED_STORAGES_BASE ------------------- - Default:: { @@ -285,36 +275,39 @@ Default:: 'ftp': 'scrapy.extensions.feedexport.FTPFeedStorage', } -A dict containing the built-in feed storage backends supported by Scrapy. +A dict containing all feed storage backends supported by your project. The keys +are URI schemes and the values are paths to storage classes. + +When you set :setting:`FEED_STORAGES` manually, e.g. in your project's settings +module, it will be merged with the default, not overwrite it. If you want to +disable any of the default feed storage backends, you must assign ``None`` as +their value. .. setting:: FEED_EXPORTERS FEED_EXPORTERS -------------- -Default:: ``{}`` - -A dict containing additional exporters supported by your project. The keys are -URI schemes and the values are paths to :ref:`Item exporter ` -classes. - -.. setting:: FEED_EXPORTERS_BASE - -FEED_EXPORTERS_BASE -------------------- - Default:: - FEED_EXPORTERS_BASE = { + { 'json': 'scrapy.exporters.JsonItemExporter', 'jsonlines': 'scrapy.exporters.JsonLinesItemExporter', + 'jl': 'scrapy.exporters.JsonLinesItemExporter', 'csv': 'scrapy.exporters.CsvItemExporter', 'xml': 'scrapy.exporters.XmlItemExporter', 'marshal': 'scrapy.exporters.MarshalItemExporter', + 'pickle': 'scrapy.exporters.PickleItemExporter', } -A dict containing the built-in feed exporters supported by Scrapy. +A dict containing all feed exporters supported by your project. The keys are +URI schemes and the values are paths to :ref:`Item exporter ` +classes. +When you set :setting:`FEED_EXPORTERS` manually, e.g. in your project's settings +module, it will be merged with the default, not overwrite it. If you want to +disable any of the default feed exporters, you must assign ``None`` as their +value. .. _URI: http://en.wikipedia.org/wiki/Uniform_Resource_Identifier .. _Amazon S3: http://aws.amazon.com/s3/ diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 484065406..642f4eb84 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -269,6 +269,11 @@ Default:: The default headers used for Scrapy HTTP Requests. They're populated in the :class:`~scrapy.downloadermiddlewares.defaultheaders.DefaultHeadersMiddleware`. +When you set :setting:`DEFAULT_REQUEST_HEADERS` manually, e.g. in your +project's settings module, it will be merged with the default, not overwrite it. +If you want to disable any of the default request headers (and not replace them) +you must assign ``None`` as their value. + .. setting:: DEPTH_LIMIT DEPTH_LIMIT @@ -350,16 +355,6 @@ The downloader to use for crawling. DOWNLOADER_MIDDLEWARES ---------------------- -Default:: ``{}`` - -A dict containing the downloader middlewares enabled in your project, and their -orders. For more info see :ref:`topics-downloader-middleware-setting`. - -.. setting:: DOWNLOADER_MIDDLEWARES_BASE - -DOWNLOADER_MIDDLEWARES_BASE ---------------------------- - Default:: { @@ -369,6 +364,7 @@ Default:: 'scrapy.downloadermiddlewares.useragent.UserAgentMiddleware': 400, 'scrapy.downloadermiddlewares.retry.RetryMiddleware': 500, 'scrapy.downloadermiddlewares.defaultheaders.DefaultHeadersMiddleware': 550, + 'scrapy.downloadermiddlewares.ajaxcrawl.AjaxCrawlMiddleware': 560, 'scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware': 580, 'scrapy.downloadermiddlewares.httpcompression.HttpCompressionMiddleware': 590, 'scrapy.downloadermiddlewares.redirect.RedirectMiddleware': 600, @@ -379,10 +375,16 @@ Default:: 'scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware': 900, } -A dict containing the downloader middlewares enabled by default in Scrapy. You -should never modify this setting in your project, modify -:setting:`DOWNLOADER_MIDDLEWARES` instead. For more info see -:ref:`topics-downloader-middleware-setting`. +A dict containing the downloader middlewares enabled in your project, and their +orders. Low orders are closer to the engine, high orders are closer to the +downloader. + +When you set :setting:`DOWNLOADER_MIDDLEWARES` manually, e.g. in your project's +settings module, it will be merged with the default, not overwrite it. If you +want to disable any of the default downloader middlewares you must assign +``None`` as their value. + +For more info see :ref:`topics-downloader-middleware-setting`. .. setting:: DOWNLOADER_STATS @@ -423,33 +425,23 @@ spider attribute. DOWNLOAD_HANDLERS ----------------- -Default: ``{}`` - -A dict containing the request downloader handlers enabled in your project. -See `DOWNLOAD_HANDLERS_BASE` for example format. - -.. setting:: DOWNLOAD_HANDLERS_BASE - -DOWNLOAD_HANDLERS_BASE ----------------------- - Default:: { 'file': 'scrapy.core.downloader.handlers.file.FileDownloadHandler', - 'http': 'scrapy.core.downloader.handlers.http.HttpDownloadHandler', - 'https': 'scrapy.core.downloader.handlers.http.HttpDownloadHandler', + 'http': 'scrapy.core.downloader.handlers.http.HTTPDownloadHandler', + 'https': 'scrapy.core.downloader.handlers.http.HTTPDownloadHandler', 's3': 'scrapy.core.downloader.handlers.s3.S3DownloadHandler', + 'ftp': 'scrapy.core.downloader.handlers.ftp.FTPDownloadHandler', } -A dict containing the request download handlers enabled by default in Scrapy. -You should never modify this setting in your project, modify -:setting:`DOWNLOAD_HANDLERS` instead. -If you want to disable any of the above download handlers you must define them -in your project's :setting:`DOWNLOAD_HANDLERS` setting and assign `None` -as their value. For example, if you want to disable the file download -handler:: +A dict containing the request downloader handlers enabled in your project. + +When you set :setting:`DOWNLOAD_HANDLERS` manually, e.g. in your project's +settings module, it will be merged with the default, not overwrite it. If you +want to disable any of the default download handlers you must assign ``None`` +as their value. For example, if you want to disable the file download handler:: DOWNLOAD_HANDLERS = { 'file': None, @@ -552,15 +544,6 @@ to ``vi`` (on Unix systems) or the IDLE editor (on Windows). EXTENSIONS ---------- -Default:: ``{}`` - -A dict containing the extensions enabled in your project, and their orders. - -.. setting:: EXTENSIONS_BASE - -EXTENSIONS_BASE ---------------- - Default:: { @@ -575,13 +558,19 @@ Default:: 'scrapy.extensions.throttle.AutoThrottle': 0, } -The list of available extensions. Keep in mind that some of them need to -be enabled through a setting. By default, this setting contains all stable -built-in extensions. +A dict containing the extensions enabled in your project, and their orders. By +default, this setting contains all stable built-in extensions. Keep in mind that +some of them need to be enabled through a setting. + +When you set :setting:`EXTENSIONS` manually, e.g. in your project's settings +module, it will be merged with the default, not overwrite it. If you want to +disable any of the default enabled extensions you must assign ``None`` as their +value. For more information See the :ref:`extensions user guide ` and the :ref:`list of available extensions `. + .. setting:: ITEM_PIPELINES ITEM_PIPELINES @@ -589,12 +578,9 @@ ITEM_PIPELINES Default: ``{}`` -A dict containing the item pipelines to use, and their orders. The dict is -empty by default order values are arbitrary but it's customary to define them -in the 0-1000 range. - -Lists are supported in :setting:`ITEM_PIPELINES` for backwards compatibility, -but they are deprecated. +A dict containing the item pipelines to use, and their orders. Order values are +arbitrary, but it is customary to define them in the 0-1000 range. Lower orders +process before higher orders. Example:: @@ -603,16 +589,6 @@ Example:: 'mybot.pipelines.validate.StoreMyItem': 800, } -.. setting:: ITEM_PIPELINES_BASE - -ITEM_PIPELINES_BASE -------------------- - -Default: ``{}`` - -A dict containing the pipelines enabled by default in Scrapy. You should never -modify this setting in your project, modify :setting:`ITEM_PIPELINES` instead. - .. setting:: LOG_ENABLED LOG_ENABLED @@ -638,7 +614,7 @@ LOG_FILE Default: ``None`` -File name to use for logging output. If None, standard error will be used. +File name to use for logging output. If ``None``, standard error will be used. .. setting:: LOG_FORMAT @@ -902,16 +878,6 @@ The scheduler to use for crawling. SPIDER_CONTRACTS ---------------- -Default:: ``{}`` - -A dict containing the scrapy contracts enabled in your project, used for -testing spiders. For more info see :ref:`topics-contracts`. - -.. setting:: SPIDER_CONTRACTS_BASE - -SPIDER_CONTRACTS_BASE ---------------------- - Default:: { @@ -920,9 +886,13 @@ Default:: 'scrapy.contracts.default.ScrapesContract': 3, } -A dict containing the scrapy contracts enabled by default in Scrapy. You should -never modify this setting in your project, modify :setting:`SPIDER_CONTRACTS` -instead. For more info see :ref:`topics-contracts`. +A dict containing the scrapy contracts enabled in your project, used for +testing spiders. For more info see :ref:`topics-contracts`. + +When you set :setting:`SPIDER_CONTRACTS` manually, e.g. in your project's +settings module, it will be merged with the default, not overwrite it. If you +want to disable any of the default contracts you must assign ``None`` as their +value. .. setting:: SPIDER_LOADER_CLASS @@ -939,16 +909,6 @@ The class that will be used for loading spiders, which must implement the SPIDER_MIDDLEWARES ------------------ -Default:: ``{}`` - -A dict containing the spider middlewares enabled in your project, and their -orders. For more info see :ref:`topics-spider-middleware-setting`. - -.. setting:: SPIDER_MIDDLEWARES_BASE - -SPIDER_MIDDLEWARES_BASE ------------------------ - Default:: { @@ -959,10 +919,14 @@ Default:: 'scrapy.spidermiddlewares.depth.DepthMiddleware': 900, } -A dict containing the spider middlewares enabled by default in Scrapy. You -should never modify this setting in your project, modify -:setting:`SPIDER_MIDDLEWARES` instead. For more info see -:ref:`topics-spider-middleware-setting`. +A dict containing the spider middlewares enabled in your project, and their +orders. Low orders are closer to the engine, high orders are closer to the +spider. For more info see :ref:`topics-spider-middleware-setting`. + +When you set :setting:`SPIDER_MIDDLEWARES` manually, e.g. in your project's +settings module, it will be merged with the default, not overwrite it. If you +want to disable any of the default spider middlewares you must assign ``None`` +as their value. .. setting:: SPIDER_MODULES diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 84daaaa55..d448801d3 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -24,22 +24,20 @@ Here's an example:: 'myproject.middlewares.CustomSpiderMiddleware': 543, } -The :setting:`SPIDER_MIDDLEWARES` setting is merged with the -:setting:`SPIDER_MIDDLEWARES_BASE` setting defined in Scrapy (and not meant to -be overridden) and then sorted by order to get the final sorted list of enabled -middlewares: the first middleware is the one closer to the engine and the last -is the one closer to the spider. +The specified :setting:`SPIDER_MIDDLEWARES` setting is merged with the default +one (i.e. it does not overwrite it) and then sorted by order to get the final +sorted list of enabled middlewares: the first middleware is the one closer to +the engine and the last is the one closer to the spider. -To decide which order to assign to your middleware see the -:setting:`SPIDER_MIDDLEWARES_BASE` setting and pick a value according to where +To decide which order to assign to your middleware see the default +:setting:`SPIDER_MIDDLEWARES` setting and pick a value according to where you want to insert the middleware. The order does matter because each middleware performs a different action and your middleware could depend on some previous (or subsequent) middleware being applied. -If you want to disable a builtin middleware (the ones defined in -:setting:`SPIDER_MIDDLEWARES_BASE`, and enabled by default) you must define it -in your project :setting:`SPIDER_MIDDLEWARES` setting and assign `None` as its -value. For example, if you want to disable the off-site middleware:: +If you want to disable a builtin middleware you must define it in your project's +:setting:`SPIDER_MIDDLEWARES` setting and assign ``None`` as its value. For +example, if you want to disable the off-site middleware:: SPIDER_MIDDLEWARES = { 'myproject.middlewares.CustomSpiderMiddleware': 543, @@ -173,7 +171,7 @@ information on how to use them and how to write your own spider middleware, see the :ref:`spider middleware usage guide `. For a list of the components enabled by default (and their orders) see the -:setting:`SPIDER_MIDDLEWARES_BASE` setting. +:setting:`SPIDER_MIDDLEWARES` setting. DepthMiddleware --------------- diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py index 017595f04..a423ba2c9 100644 --- a/scrapy/commands/check.py +++ b/scrapy/commands/check.py @@ -58,10 +58,7 @@ class Command(ScrapyCommand): def run(self, args, opts): # load contracts - contracts = build_component_list( - self.settings['SPIDER_CONTRACTS_BASE'], - self.settings['SPIDER_CONTRACTS'], - ) + contracts = build_component_list(self.settings._getcomposite('SPIDER_CONTRACTS')) conman = ContractsManager(load_object(c) for c in contracts) runner = TextTestRunner(verbosity=2 if opts.verbose else 1) result = TextTestResult(runner.stream, runner.descriptions, runner.verbosity) diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index 72df11476..9c8a3d4ce 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -1,6 +1,6 @@ import os from scrapy.commands import ScrapyCommand -from scrapy.utils.conf import arglist_to_dict +from scrapy.utils.conf import arglist_to_dict, remove_none_values from scrapy.exceptions import UsageError @@ -34,10 +34,8 @@ class Command(ScrapyCommand): self.settings.set('FEED_URI', 'stdout:', priority='cmdline') else: self.settings.set('FEED_URI', opts.output, priority='cmdline') - valid_output_formats = ( - list(self.settings.getdict('FEED_EXPORTERS').keys()) + - list(self.settings.getdict('FEED_EXPORTERS_BASE').keys()) - ) + feed_exporters = remove_none_values(self.settings._getcomposite('FEED_EXPORTERS')) + valid_output_formats = feed_exporters.keys() if not opts.output_format: opts.output_format = os.path.splitext(opts.output)[1].replace(".", "") if opts.output_format not in valid_output_formats: diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 88f5a3015..7d85984c3 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -5,7 +5,7 @@ from importlib import import_module from scrapy.utils.spider import iter_spider_classes from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError -from scrapy.utils.conf import arglist_to_dict +from scrapy.utils.conf import arglist_to_dict, remove_none_values def _import_file(filepath): @@ -57,10 +57,8 @@ class Command(ScrapyCommand): self.settings.set('FEED_URI', 'stdout:', priority='cmdline') else: self.settings.set('FEED_URI', opts.output, priority='cmdline') - valid_output_formats = ( - list(self.settings.getdict('FEED_EXPORTERS').keys()) + - list(self.settings.getdict('FEED_EXPORTERS_BASE').keys()) - ) + feed_exporters = remove_none_values(self.settings._getcomposite('FEED_EXPORTERS')) + valid_output_formats = feed_exporters.keys() if not opts.output_format: opts.output_format = os.path.splitext(opts.output)[1].replace(".", "") if opts.output_format not in valid_output_formats: diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 6c9514af6..9b118c39b 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -4,6 +4,7 @@ import logging from twisted.internet import defer import six from scrapy.exceptions import NotSupported, NotConfigured +from scrapy.utils.conf import remove_none_values from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import load_object from scrapy import signals @@ -19,13 +20,8 @@ class DownloadHandlers(object): self._schemes = {} # stores acceptable schemes on instancing self._handlers = {} # stores instanced handlers for schemes self._notconfigured = {} # remembers failed handlers - handlers = crawler.settings.get('DOWNLOAD_HANDLERS_BASE') - handlers.update(crawler.settings.get('DOWNLOAD_HANDLERS', {})) + handlers = remove_none_values(crawler.settings._getcomposite('DOWNLOAD_HANDLERS')) for scheme, clspath in six.iteritems(handlers): - # Allow to disable a handler just like any other - # component (extension, middleware, etc). - if clspath is None: - continue self._schemes[scheme] = clspath crawler.signals.connect(self._close, signals.engine_stopped) diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 9cd30c144..958113fc3 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -19,8 +19,7 @@ class DownloaderMiddlewareManager(MiddlewareManager): @classmethod def _get_mwlist_from_settings(cls, settings): - return build_component_list(settings['DOWNLOADER_MIDDLEWARES_BASE'], \ - settings['DOWNLOADER_MIDDLEWARES']) + return build_component_list(settings._getcomposite('DOWNLOADER_MIDDLEWARES')) def _add_middleware(self, mw): if hasattr(mw, 'process_request'): diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index c1c5b10fc..b5c80c350 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -18,8 +18,7 @@ class SpiderMiddlewareManager(MiddlewareManager): @classmethod def _get_mwlist_from_settings(cls, settings): - return build_component_list(settings['SPIDER_MIDDLEWARES_BASE'], \ - settings['SPIDER_MIDDLEWARES']) + return build_component_list(settings._getcomposite('SPIDER_MIDDLEWARES')) def _add_middleware(self, mw): super(SpiderMiddlewareManager, self)._add_middleware(mw) diff --git a/scrapy/downloadermiddlewares/defaultheaders.py b/scrapy/downloadermiddlewares/defaultheaders.py index f1d2bd631..c8924c04a 100644 --- a/scrapy/downloadermiddlewares/defaultheaders.py +++ b/scrapy/downloadermiddlewares/defaultheaders.py @@ -4,6 +4,8 @@ DefaultHeaders downloader middleware See documentation in docs/topics/downloader-middleware.rst """ +from scrapy.utils.conf import remove_none_values + class DefaultHeadersMiddleware(object): @@ -12,7 +14,8 @@ class DefaultHeadersMiddleware(object): @classmethod def from_crawler(cls, crawler): - return cls(crawler.settings.get('DEFAULT_REQUEST_HEADERS').items()) + headers = remove_none_values(crawler.settings['DEFAULT_REQUEST_HEADERS']) + return cls(headers.items()) def process_request(self, request, spider): for k, v in self._headers: diff --git a/scrapy/extension.py b/scrapy/extension.py index f68b1ba68..4ceb32c68 100644 --- a/scrapy/extension.py +++ b/scrapy/extension.py @@ -12,5 +12,4 @@ class ExtensionManager(MiddlewareManager): @classmethod def _get_mwlist_from_settings(cls, settings): - return build_component_list(settings['EXTENSIONS_BASE'], \ - settings['EXTENSIONS']) + return build_component_list(settings._getcomposite('EXTENSIONS')) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 7560e89d3..fb07657d6 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -18,6 +18,7 @@ from twisted.internet import defer, threads from w3lib.url import file_uri_to_path from scrapy import signals +from scrapy.utils.conf import remove_none_values from scrapy.utils.ftp import ftp_makedirs_cwd from scrapy.exceptions import NotConfigured from scrapy.utils.misc import load_object @@ -195,8 +196,7 @@ class FeedExporter(object): return item def _load_components(self, setting_prefix): - conf = dict(self.settings['%s_BASE' % setting_prefix]) - conf.update(self.settings[setting_prefix]) + conf = remove_none_values(self.settings._getcomposite(setting_prefix)) d = {} for k, v in conf.items(): try: diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index d433498f5..8df0d3154 100644 --- a/scrapy/pipelines/__init__.py +++ b/scrapy/pipelines/__init__.py @@ -13,15 +13,7 @@ class ItemPipelineManager(MiddlewareManager): @classmethod def _get_mwlist_from_settings(cls, settings): - item_pipelines = settings['ITEM_PIPELINES'] - if isinstance(item_pipelines, (tuple, list, set, frozenset)): - from scrapy.exceptions import ScrapyDeprecationWarning - import warnings - warnings.warn('ITEM_PIPELINES defined as a list or a set is deprecated, switch to a dict', - category=ScrapyDeprecationWarning, stacklevel=1) - # convert old ITEM_PIPELINE list to a dict with order 500 - item_pipelines = dict(zip(item_pipelines, range(500, 500+len(item_pipelines)))) - return build_component_list(settings['ITEM_PIPELINES_BASE'], item_pipelines) + return build_component_list(settings._getcomposite('ITEM_PIPELINES')) def _add_middleware(self, pipe): super(ItemPipelineManager, self)._add_middleware(pipe) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index fa7fa3178..7eea562e1 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -36,13 +36,21 @@ class SettingsAttribute(object): def __init__(self, value, priority): self.value = value - self.priority = priority + if isinstance(self.value, BaseSettings): + self.priority = max(self.value.maxpriority(), priority) + else: + self.priority = priority def set(self, value, priority): """Sets value if priority is higher or equal than current priority.""" - if priority >= self.priority: - self.value = value - self.priority = priority + if isinstance(self.value, BaseSettings): + # Ignore self.priority if self.value has per-key priorities + self.value.update(value, priority) + self.priority = max(self.value.maxpriority(), priority) + else: + if priority >= self.priority: + self.value = value + self.priority = priority def __str__(self): return " Date: Thu, 2 Jul 2015 16:51:15 +0200 Subject: [PATCH 3/8] Move Settings documentation to docstrings --- docs/topics/api.rst | 209 ++---------------------------------- scrapy/settings/__init__.py | 193 ++++++++++++++++++++++++++++++++- 2 files changed, 195 insertions(+), 207 deletions(-) diff --git a/docs/topics/api.rst b/docs/topics/api.rst index 923bd80b0..42c0133c1 100644 --- a/docs/topics/api.rst +++ b/docs/topics/api.rst @@ -140,211 +140,14 @@ Settings API For a detailed explanation on each settings sources, see: :ref:`topics-settings`. -.. function:: get_settings_priority(priority) +.. autofunction:: get_settings_priority - Small helper function that looks up a given string priority in the - :attr:`~scrapy.settings.SETTINGS_PRIORITIES` dictionary and returns its - numerical value, or directly returns a given numerical priority. +.. autoclass:: Settings + :show-inheritance: + :members: -.. class:: Settings(values={}, priority='project') - - This object stores Scrapy settings for the configuration of internal - components, and can be used for any further customization. - - It is a direct subclass and supports all methods of - :class:`~scrapy.settings.BaseSettings`. Additionally, after instantiation - of this class, the new object will have the global default settings - described on :ref:`topics-settings-ref` already populated. - -.. class:: BaseSettings(values={}, priority='project') - - Instances of this class behave like dictionaries, but store priorities - along with their ``(key, value)`` pairs, and can be frozen (i.e. marked - immutable). - - Key-value entries can be passed on initialization with the ``values`` - argument, and they would take the ``priority`` level (unless ``values`` is - already an instance of :class:`~scrapy.settings.BaseSettings`, in which - case the existing priority levels will be kept). If the ``priority`` - argument is a string, the priority name will be looked up in - :attr:`~scrapy.settings.SETTINGS_PRIORITIES`. Otherwise, a specific integer - should be provided. - - Once the object is created, new settings can be loaded or updated with the - :meth:`~scrapy.settings.BaseSettings.set` method, and can be accessed with - the square bracket notation of dictionaries, or with the - :meth:`~scrapy.settings.BaseSettings.get` method of the instance and its - value conversion variants. When requesting a stored key, the value with the - highest priority will be retrieved. - - .. method:: set(name, value, priority='project') - - Store a key/value attribute with a given priority. - - Settings should be populated *before* configuring the Crawler object - (through the :meth:`~scrapy.crawler.Crawler.configure` method), - otherwise they won't have any effect. - - :param name: the setting name - :type name: string - - :param value: the value to associate with the setting - :type value: any - - :param priority: the priority of the setting. Should be a key of - :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer - :type priority: string or int - - .. method:: update(values, priority='project') - - Store key/value pairs with a given priority. - - This is a helper function that calls - :meth:`~scrapy.settings.BaseSettings.set` for every item of ``values`` - with the provided ``priority``. - - If ``values`` is a string, it is assumed to be JSON-encoded and parsed - into a dict with ``json.loads()`` first. If it is a - :class:`~scrapy.settings.BaseSettings` instance, the per-key priorities - will be used and the ``priority`` parameter ignored. This allows - inserting/updating settings with different priorities with a single - command. - - :param values: the settings names and values - :type values: dict or string or :class:`~scrapy.settings.BaseSettings` - - :param priority: the priority of the settings. Should be a key of - :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer - :type priority: string or int - - .. method:: setmodule(module, priority='project') - - Store settings from a module with a given priority. - - This is a helper function that calls - :meth:`~scrapy.settings.BaseSettings.set` for every globally declared - uppercase variable of ``module`` with the provided ``priority``. - - :param module: the module or the path of the module - :type module: module object or string - - :param priority: the priority of the settings. Should be a key of - :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer - :type priority: string or int - - .. method:: get(name, default=None) - - Get a setting value without affecting its original type. - - :param name: the setting name - :type name: string - - :param default: the value to return if no setting is found - :type default: any - - .. method:: getbool(name, default=False) - - Get a setting value as a boolean. For example, both ``1`` and ``'1'``, and - ``True`` return ``True``, while ``0``, ``'0'``, ``False`` and ``None`` - return ``False```` - - For example, settings populated through environment variables set to ``'0'`` - will return ``False`` when using this method. - - :param name: the setting name - :type name: string - - :param default: the value to return if no setting is found - :type default: any - - .. method:: getint(name, default=0) - - Get a setting value as an int - - :param name: the setting name - :type name: string - - :param default: the value to return if no setting is found - :type default: any - - .. method:: getfloat(name, default=0.0) - - Get a setting value as a float - - :param name: the setting name - :type name: string - - :param default: the value to return if no setting is found - :type default: any - - .. method:: getlist(name, default=None) - - Get a setting value as a list. If the setting original type is a list, a - copy of it will be returned. If it's a string it will be split by ",". - - For example, settings populated through environment variables set to - ``'one,two'`` will return a list ['one', 'two'] when using this method. - - :param name: the setting name - :type name: string - - :param default: the value to return if no setting is found - :type default: any - - .. method:: getdict(name, default=None) - - Get a setting value as a dictionary. If the setting original type is a - dictionary, a copy of it will be returned. If it is a string it will be - evaluated as a JSON dictionary. In the case that it is a - :class:`~scrapy.settings.BaseSettings` instance itself, it will be - converted to a dictionary, containing all its current settings values - as they would be returned by :meth:`~scrapy.settings.BaseSettings.get`, - and losing all information about priority and mutability. - - :param name: the setting name - :type name: string - - :param default: the value to return if no setting is found - :type default: any - - .. method:: copy() - - Make a deep copy of current settings. - - This method returns a new instance of the :class:`Settings` class, - populated with the same values and their priorities. - - Modifications to the new object won't be reflected on the original - settings. - - .. method:: freeze() - - Disable further changes to the current settings. - - After calling this method, the present state of the settings will become - immutable. Trying to change values through the :meth:`~set` method and - its variants won't be possible and will be alerted. - - .. method:: frozencopy() - - Return an immutable copy of the current settings. - - Alias for a :meth:`~freeze` call in the object returned by :meth:`copy` - - .. method:: getpriority(name) - - Return the current numerical priority value of a setting, or ``None`` if - the given ``name`` does not exist. - - :param name: the setting name - :type name: string - - .. method:: maxpriority() - - Return the numerical value of the highest priority present throughout - all settings, or the numerical value for ``default`` from - :attr:`~scrapy.settings.SETTINGS_PRIORITIES` if there are no settings - stored. +.. autoclass:: BaseSettings + :members: .. _topics-api-spiderloader: diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 7eea562e1..1216aabcb 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -20,6 +20,11 @@ SETTINGS_PRIORITIES = { } def get_settings_priority(priority): + """ + Small helper function that looks up a given string priority in the + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` dictionary and returns its + numerical value, or directly returns a given numerical priority. + """ if isinstance(priority, six.string_types): return SETTINGS_PRIORITIES[priority] else: @@ -60,6 +65,26 @@ class SettingsAttribute(object): class BaseSettings(MutableMapping): + """ + Instances of this class behave like dictionaries, but store priorities + along with their ``(key, value)`` pairs, and can be frozen (i.e. marked + immutable). + + Key-value entries can be passed on initialization with the ``values`` + argument, and they would take the ``priority`` level (unless ``values`` is + already an instance of :class:`~scrapy.settings.BaseSettings`, in which + case the existing priority levels will be kept). If the ``priority`` + argument is a string, the priority name will be looked up in + :attr:`~scrapy.settings.SETTINGS_PRIORITIES`. Otherwise, a specific integer + should be provided. + + Once the object is created, new settings can be loaded or updated with the + :meth:`~scrapy.settings.BaseSettings.set` method, and can be accessed with + the square bracket notation of dictionaries, or with the + :meth:`~scrapy.settings.BaseSettings.get` method of the instance and its + value conversion variants. When requesting a stored key, the value with the + highest priority will be retrieved. + """ def __init__(self, values=None, priority='project'): self.frozen = False @@ -76,28 +101,94 @@ class BaseSettings(MutableMapping): return name in self.attributes def get(self, name, default=None): + """ + Get a setting value without affecting its original type. + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any + """ return self[name] if self[name] is not None else default def getbool(self, name, default=False): """ - True is: 1, '1', True - False is: 0, '0', False, None + Get a setting value as a boolean. + + ``1``, ``'1'``, and ``True`` return ``True``, while ``0``, ``'0'``, + ``False`` and ``None`` return ``False``. + + For example, settings populated through environment variables set to + ``'0'`` will return ``False`` when using this method. + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any """ return bool(int(self.get(name, default))) def getint(self, name, default=0): + """ + Get a setting value as an int. + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any + """ return int(self.get(name, default)) def getfloat(self, name, default=0.0): + """ + Get a setting value as a float. + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any + """ return float(self.get(name, default)) def getlist(self, name, default=None): + """ + Get a setting value as a list. If the setting original type is a list, a + copy of it will be returned. If it's a string it will be split by ",". + + For example, settings populated through environment variables set to + ``'one,two'`` will return a list ['one', 'two'] when using this method. + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any + """ value = self.get(name, default or []) if isinstance(value, six.string_types): value = value.split(',') return list(value) def getdict(self, name, default=None): + """ + Get a setting value as a dictionary. If the setting original type is a + dictionary, a copy of it will be returned. If it is a string it will be + evaluated as a JSON dictionary. In the case that it is a + :class:`~scrapy.settings.BaseSettings` instance itself, it will be + converted to a dictionary, containing all its current settings values + as they would be returned by :meth:`~scrapy.settings.BaseSettings.get`, + and losing all information about priority and mutability. + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any + """ value = self.get(name, default or {}) if isinstance(value, six.string_types): value = json.loads(value) @@ -118,12 +209,25 @@ class BaseSettings(MutableMapping): return self[name] def getpriority(self, name): + """ + Return the current numerical priority value of a setting, or ``None`` if + the given ``name`` does not exist. + + :param name: the setting name + :type name: string + """ prio = None if name in self: prio = self.attributes[name].priority return prio def maxpriority(self): + """ + Return the numerical value of the highest priority present throughout + all settings, or the numerical value for ``default`` from + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` if there are no settings + stored. + """ if len(self) > 0: return max(self.getpriority(name) for name in self) else: @@ -133,6 +237,23 @@ class BaseSettings(MutableMapping): self.set(name, value) def set(self, name, value, priority='project'): + """ + Store a key/value attribute with a given priority. + + Settings should be populated *before* configuring the Crawler object + (through the :meth:`~scrapy.crawler.Crawler.configure` method), + otherwise they won't have any effect. + + :param name: the setting name + :type name: string + + :param value: the value to associate with the setting + :type value: any + + :param priority: the priority of the setting. Should be a key of + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer + :type priority: string or int + """ self._assert_mutability() priority = get_settings_priority(priority) if name not in self: @@ -147,6 +268,20 @@ class BaseSettings(MutableMapping): self.update(values, priority) def setmodule(self, module, priority='project'): + """ + Store settings from a module with a given priority. + + This is a helper function that calls + :meth:`~scrapy.settings.BaseSettings.set` for every globally declared + uppercase variable of ``module`` with the provided ``priority``. + + :param module: the module or the path of the module + :type module: module object or string + + :param priority: the priority of the settings. Should be a key of + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer + :type priority: string or int + """ self._assert_mutability() if isinstance(module, six.string_types): module = import_module(module) @@ -155,6 +290,27 @@ class BaseSettings(MutableMapping): self.set(key, getattr(module, key), priority) def update(self, values, priority='project'): + """ + Store key/value pairs with a given priority. + + This is a helper function that calls + :meth:`~scrapy.settings.BaseSettings.set` for every item of ``values`` + with the provided ``priority``. + + If ``values`` is a string, it is assumed to be JSON-encoded and parsed + into a dict with ``json.loads()`` first. If it is a + :class:`~scrapy.settings.BaseSettings` instance, the per-key priorities + will be used and the ``priority`` parameter ignored. This allows + inserting/updating settings with different priorities with a single + command. + + :param values: the settings names and values + :type values: dict or string or :class:`~scrapy.settings.BaseSettings` + + :param priority: the priority of the settings. Should be a key of + :attr:`~scrapy.settings.SETTINGS_PRIORITIES` or an integer + :type priority: string or int + """ self._assert_mutability() if isinstance(values, six.string_types): values = json.loads(values) @@ -181,12 +337,33 @@ class BaseSettings(MutableMapping): raise TypeError("Trying to modify an immutable Settings object") def copy(self): + """ + Make a deep copy of current settings. + + This method returns a new instance of the :class:`Settings` class, + populated with the same values and their priorities. + + Modifications to the new object won't be reflected on the original + settings. + """ return copy.deepcopy(self) def freeze(self): + """ + Disable further changes to the current settings. + + After calling this method, the present state of the settings will become + immutable. Trying to change values through the :meth:`~set` method and + its variants won't be possible and will be alerted. + """ self.frozen = True def frozencopy(self): + """ + Return an immutable copy of the current settings. + + Alias for a :meth:`~freeze` call in the object returned by :meth:`copy`. + """ copy = self.copy() copy.freeze() return copy @@ -252,6 +429,15 @@ class _DictProxy(MutableMapping): class Settings(BaseSettings): + """ + This object stores Scrapy settings for the configuration of internal + components, and can be used for any further customization. + + It is a direct subclass and supports all methods of + :class:`~scrapy.settings.BaseSettings`. Additionally, after instantiation + of this class, the new object will have the global default settings + described on :ref:`topics-settings-ref` already populated. + """ def __init__(self, values=None, priority='project'): # Do not pass kwarg values here. We don't want to promote user-defined @@ -261,8 +447,7 @@ class Settings(BaseSettings): self.setmodule(default_settings, 'default') # Promote default dictionaries to BaseSettings instances for per-key # priorities - for name in self: - val = self[name] + for name, val in six.iteritems(self): if isinstance(val, dict): self.set(name, BaseSettings(val, 'default'), 'default') self.update(values, priority) From 9bd7af8a625d63a3372346bf6c69c80a2a9832a8 Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Tue, 25 Aug 2015 23:41:34 +0200 Subject: [PATCH 4/8] Remove unused import in scrapy.settings --- scrapy/settings/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 1216aabcb..ed201e980 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -2,7 +2,7 @@ import six import json import copy import warnings -from collections import Mapping, MutableMapping +from collections import MutableMapping from importlib import import_module from scrapy.utils.deprecate import create_deprecated_class From 9eb3597d159a8556259946b7acf39d3c368113dc Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Tue, 25 Aug 2015 23:43:54 +0200 Subject: [PATCH 5/8] PEP8ify settings module --- scrapy/settings/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index ed201e980..6c922a709 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -19,6 +19,7 @@ SETTINGS_PRIORITIES = { 'cmdline': 40, } + def get_settings_priority(priority): """ Small helper function that looks up a given string priority in the @@ -196,8 +197,8 @@ class BaseSettings(MutableMapping): def _getcomposite(self, name): # DO NOT USE THIS FUNCTION IN YOUR CUSTOM PROJECTS - # It's for internal use in the transition away from the _BASE settings and - # will be removed along with _BASE support in a future release + # It's for internal use in the transition away from the _BASE settings + # and will be removed along with _BASE support in a future release basename = name + "_BASE" if basename in self: warnings.warn('_BASE settings are deprecated.', @@ -482,6 +483,7 @@ def iter_default_settings(): if name.isupper(): yield name, getattr(default_settings, name) + def overridden_settings(settings): """Return a dict of the settings that have been overridden""" for name, defvalue in iter_default_settings(): From 90198e5324f3172ec83c457049a7a66a2805d875 Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Tue, 25 Aug 2015 23:44:37 +0200 Subject: [PATCH 6/8] Add __repr__ method for BaseSettings --- scrapy/settings/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 6c922a709..3ae2187ae 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -378,7 +378,8 @@ class BaseSettings(MutableMapping): def __str__(self): return str(self.attributes) - __repr__ = __str__ + def __repr__(self): + return "<%s %s>" % (self.__class__.__name__, self.attributes) @property def overrides(self): From f249b309ab779b5ab518f54f309d7a4ac6661ec7 Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Wed, 26 Aug 2015 00:26:06 +0200 Subject: [PATCH 7/8] Move scrapy.utils.conf.remove_none_values to s.u.python.without_none_values --- scrapy/commands/crawl.py | 5 +++-- scrapy/commands/runspider.py | 5 +++-- scrapy/core/downloader/handlers/__init__.py | 4 ++-- scrapy/downloadermiddlewares/defaultheaders.py | 4 ++-- scrapy/extensions/feedexport.py | 4 ++-- scrapy/utils/conf.py | 9 ++------- scrapy/utils/python.py | 12 ++++++++++++ tests/test_utils_conf.py | 9 +-------- tests/test_utils_python.py | 10 +++++++++- 9 files changed, 36 insertions(+), 26 deletions(-) diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index 9c8a3d4ce..7f5c64c20 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -1,6 +1,7 @@ import os from scrapy.commands import ScrapyCommand -from scrapy.utils.conf import arglist_to_dict, remove_none_values +from scrapy.utils.conf import arglist_to_dict +from scrapy.utils.python import without_none_values from scrapy.exceptions import UsageError @@ -34,7 +35,7 @@ class Command(ScrapyCommand): self.settings.set('FEED_URI', 'stdout:', priority='cmdline') else: self.settings.set('FEED_URI', opts.output, priority='cmdline') - feed_exporters = remove_none_values(self.settings._getcomposite('FEED_EXPORTERS')) + feed_exporters = without_none_values(self.settings._getcomposite('FEED_EXPORTERS')) valid_output_formats = feed_exporters.keys() if not opts.output_format: opts.output_format = os.path.splitext(opts.output)[1].replace(".", "") diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 7d85984c3..72229bcf5 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -5,7 +5,8 @@ from importlib import import_module from scrapy.utils.spider import iter_spider_classes from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError -from scrapy.utils.conf import arglist_to_dict, remove_none_values +from scrapy.utils.conf import arglist_to_dict +from scrapy.utils.python import without_none_values def _import_file(filepath): @@ -57,7 +58,7 @@ class Command(ScrapyCommand): self.settings.set('FEED_URI', 'stdout:', priority='cmdline') else: self.settings.set('FEED_URI', opts.output, priority='cmdline') - feed_exporters = remove_none_values(self.settings._getcomposite('FEED_EXPORTERS')) + feed_exporters = without_none_values(self.settings._getcomposite('FEED_EXPORTERS')) valid_output_formats = feed_exporters.keys() if not opts.output_format: opts.output_format = os.path.splitext(opts.output)[1].replace(".", "") diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 9b118c39b..0e78e04f4 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -4,9 +4,9 @@ import logging from twisted.internet import defer import six from scrapy.exceptions import NotSupported, NotConfigured -from scrapy.utils.conf import remove_none_values from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import load_object +from scrapy.utils.python import without_none_values from scrapy import signals @@ -20,7 +20,7 @@ class DownloadHandlers(object): self._schemes = {} # stores acceptable schemes on instancing self._handlers = {} # stores instanced handlers for schemes self._notconfigured = {} # remembers failed handlers - handlers = remove_none_values(crawler.settings._getcomposite('DOWNLOAD_HANDLERS')) + handlers = without_none_values(crawler.settings._getcomposite('DOWNLOAD_HANDLERS')) for scheme, clspath in six.iteritems(handlers): self._schemes[scheme] = clspath diff --git a/scrapy/downloadermiddlewares/defaultheaders.py b/scrapy/downloadermiddlewares/defaultheaders.py index c8924c04a..93fe97673 100644 --- a/scrapy/downloadermiddlewares/defaultheaders.py +++ b/scrapy/downloadermiddlewares/defaultheaders.py @@ -4,7 +4,7 @@ DefaultHeaders downloader middleware See documentation in docs/topics/downloader-middleware.rst """ -from scrapy.utils.conf import remove_none_values +from scrapy.utils.python import without_none_values class DefaultHeadersMiddleware(object): @@ -14,7 +14,7 @@ class DefaultHeadersMiddleware(object): @classmethod def from_crawler(cls, crawler): - headers = remove_none_values(crawler.settings['DEFAULT_REQUEST_HEADERS']) + headers = without_none_values(crawler.settings['DEFAULT_REQUEST_HEADERS']) return cls(headers.items()) def process_request(self, request, spider): diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index fb07657d6..1e27a1e7e 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -18,11 +18,11 @@ from twisted.internet import defer, threads from w3lib.url import file_uri_to_path from scrapy import signals -from scrapy.utils.conf import remove_none_values from scrapy.utils.ftp import ftp_makedirs_cwd from scrapy.exceptions import NotConfigured from scrapy.utils.misc import load_object from scrapy.utils.log import failure_to_exc_info +from scrapy.utils.python import without_none_values logger = logging.getLogger(__name__) @@ -196,7 +196,7 @@ class FeedExporter(object): return item def _load_components(self, setting_prefix): - conf = remove_none_values(self.settings._getcomposite(setting_prefix)) + conf = without_none_values(self.settings._getcomposite(setting_prefix)) d = {} for k, v in conf.items(): try: diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index 80c644657..57f2b6322 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -1,6 +1,5 @@ import os import sys -import warnings from operator import itemgetter import six @@ -8,6 +7,7 @@ from six.moves.configparser import SafeConfigParser from scrapy.settings import BaseSettings from scrapy.utils.deprecate import update_classpath +from scrapy.utils.python import without_none_values def build_component_list(compdict, convert=update_classpath): @@ -37,15 +37,10 @@ def build_component_list(compdict, convert=update_classpath): if isinstance(compdict, (list, tuple)): _check_components(compdict) return type(compdict)(convert(c) for c in compdict) - compdict = remove_none_values(_map_keys(compdict)) + compdict = without_none_values(_map_keys(compdict)) return [k for k, v in sorted(six.iteritems(compdict), key=itemgetter(1))] -def remove_none_values(compdict): - """Return dict with all pairs that have value 'None' removed""" - return {k: v for k, v in six.iteritems(compdict) if v is not None} - - def arglist_to_dict(arglist): """Convert a list of arguments like ['arg1=val1', 'arg2=val2', ...] to a dict diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index d566783b2..1f9d02df5 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -324,3 +324,15 @@ def retry_on_eintr(function, *args, **kw): except IOError as e: if e.errno != errno.EINTR: raise + + +def without_none_values(iterable): + """Return a copy of `iterable` with all `None` entries removed. + + If `iterable` is a mapping, return a dictionary where all pairs that have + value `None` have been removed. + """ + try: + return {k: v for k, v in six.iteritems(iterable) if v is not None} + except AttributeError: + return type(iterable)((v for v in iterable if v is not None)) diff --git a/tests/test_utils_conf.py b/tests/test_utils_conf.py index e94ccc49b..af15d3184 100644 --- a/tests/test_utils_conf.py +++ b/tests/test_utils_conf.py @@ -1,8 +1,7 @@ import unittest from scrapy.settings import BaseSettings -from scrapy.utils.conf import (build_component_list, arglist_to_dict, - remove_none_values) +from scrapy.utils.conf import build_component_list, arglist_to_dict class BuildComponentListTest(unittest.TestCase): @@ -53,12 +52,6 @@ class BuildComponentListTest(unittest.TestCase): class UtilsConfTestCase(unittest.TestCase): - def test_remove_none_values(self): - comps = {'one': 1, 'none': None, 'three': 3, 'four': 4} - compscopy = dict(comps) - del compscopy['none'] - self.assertEqual(remove_none_values(comps), compscopy) - def test_arglist_to_dict(self): self.assertEqual(arglist_to_dict(['arg1=val1', 'arg2=val2']), {'arg1': 'val1', 'arg2': 'val2'}) diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index ca394ebf5..4f0834902 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -6,7 +6,8 @@ import six from scrapy.utils.python import ( memoizemethod_noargs, isbinarytext, equal_attributes, - WeakKeyCache, stringify_dict, get_func_args, to_bytes, to_unicode) + WeakKeyCache, stringify_dict, get_func_args, to_bytes, to_unicode, + without_none_values) __doctests__ = ['scrapy.utils.python'] @@ -212,5 +213,12 @@ class UtilsPythonTestCase(unittest.TestCase): self.assertEqual(get_func_args(" ".join), []) self.assertEqual(get_func_args(operator.itemgetter(2)), []) + def test_without_none_values(self): + self.assertEqual(without_none_values([1, None, 3, 4]), [1, 3, 4]) + self.assertEqual(without_none_values((1, None, 3, 4)), (1, 3, 4)) + self.assertEqual( + without_none_values({'one': 1, 'none': None, 'three': 3, 'four': 4}), + {'one': 1, 'three': 3, 'four': 4}) + if __name__ == "__main__": unittest.main() From 03f1720afb4a437314659a306286f440df664a0b Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Tue, 27 Oct 2015 13:56:14 +0100 Subject: [PATCH 8/8] Fix backwards-compatibility for users who explicitly set _BASE settings --- scrapy/settings/__init__.py | 18 ++++++++++++------ tests/test_settings/__init__.py | 13 +++++++++---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 3ae2187ae..13656298b 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -116,9 +116,9 @@ class BaseSettings(MutableMapping): def getbool(self, name, default=False): """ Get a setting value as a boolean. - + ``1``, ``'1'``, and ``True`` return ``True``, while ``0``, ``'0'``, - ``False`` and ``None`` return ``False``. + ``False`` and ``None`` return ``False``. For example, settings populated through environment variables set to ``'0'`` will return ``False`` when using this method. @@ -203,11 +203,17 @@ class BaseSettings(MutableMapping): if basename in self: warnings.warn('_BASE settings are deprecated.', category=ScrapyDeprecationWarning) - compsett = BaseSettings(self[name + "_BASE"], priority='default') - compsett.update(self[name]) + # When users defined a _BASE setting, they explicitly don't want to + # use any of Scrapy's defaults. Therefore, we only use these entries + # from self[name] (where the defaults now live) that have a priority + # higher than 'default' + compsett = BaseSettings(self[basename], priority='default') + for k in self[name]: + prio = self[name].getpriority(k) + if prio > get_settings_priority('default'): + compsett.set(k, self[name][k], prio) return compsett - else: - return self[name] + return self[name] def getpriority(self, name): """ diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index bb38964ef..03e7d8686 100644 --- a/tests/test_settings/__init__.py +++ b/tests/test_settings/__init__.py @@ -252,12 +252,17 @@ class BaseSettingsTest(unittest.TestCase): def test_getcomposite(self): s = BaseSettings({'TEST_BASE': {1: 1, 2: 2}, - 'TEST': BaseSettings({1: 10}), - 'HASNOBASE': BaseSettings({1: 1})}) + 'TEST': BaseSettings({1: 10, 3: 30}, 'default'), + 'HASNOBASE': BaseSettings({1: 1}, 'default')}) + s['TEST'].set(4, 4, priority='project') + # When users specify a _BASE setting they explicitly don't want to use + # Scrapy's defaults, so we don't want to see anything that has a + # 'default' priority from TEST cs = s._getcomposite('TEST') - self.assertEqual(len(cs), 2) - self.assertEqual(cs[1], 10) + self.assertEqual(len(cs), 3) + self.assertEqual(cs[1], 1) self.assertEqual(cs[2], 2) + self.assertEqual(cs[4], 4) cs = s._getcomposite('HASNOBASE') self.assertEqual(len(cs), 1) self.assertEqual(cs[1], 1)