From a769a1ef784a4383bc2f740d3a74b1e6cc6aeff9 Mon Sep 17 00:00:00 2001 From: Jakob de Maeyer Date: Fri, 19 Jun 2015 15:01:24 +0200 Subject: [PATCH] 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'} +