From 885ff6845e54511022677c1f28b84ddd4f2165dd Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Fri, 5 Sep 2014 20:06:02 +0200 Subject: [PATCH] Revert "Fixed #23384 -- Allowed overriding part of a dictionary-type setting" This reverts commit 66757fee7e921ad4c35e0b3f80c25e026100b31c. Discussions have led to think that this functionality does not bring significant benefits to justify the added complexity. Read also discussions on ticket #22734. --- django/conf/__init__.py | 7 +--- django/db/migrations/loader.py | 2 +- django/utils/datastructures.py | 20 ----------- docs/releases/1.8.txt | 3 -- docs/topics/settings.txt | 26 -------------- tests/cache/tests.py | 6 +--- tests/migrations/test_executor.py | 2 +- tests/migrations/test_loader.py | 5 +-- tests/settings_tests/tests.py | 57 ------------------------------- 9 files changed, 5 insertions(+), 123 deletions(-) diff --git a/django/conf/__init__.py b/django/conf/__init__.py index b993439018c..0b20d524840 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -12,7 +12,6 @@ import time # Needed for Windows from django.conf import global_settings from django.core.exceptions import ImproperlyConfigured -from django.utils.datastructures import dict_merge from django.utils.functional import LazyObject, empty from django.utils import six @@ -78,10 +77,6 @@ class BaseSettings(object): elif name == "ALLOWED_INCLUDE_ROOTS" and isinstance(value, six.string_types): raise ValueError("The ALLOWED_INCLUDE_ROOTS setting must be set " "to a tuple, not a string.") - elif (hasattr(self, name) and name.isupper() and - isinstance(getattr(self, name), dict) and isinstance(value, dict)): - # This allows defining only a partial dict to update a global setting - value = dict_merge(getattr(self, name), value) object.__setattr__(self, name, value) @@ -150,7 +145,7 @@ class UserSettingsHolder(BaseSettings): from the module specified in default_settings (if possible). """ self.__dict__['_deleted'] = set() - self.__dict__['default_settings'] = default_settings + self.default_settings = default_settings def __getattr__(self, name): if name in self._deleted: diff --git a/django/db/migrations/loader.py b/django/db/migrations/loader.py index 4ba3f27808d..9a3dd80d476 100644 --- a/django/db/migrations/loader.py +++ b/django/db/migrations/loader.py @@ -49,7 +49,7 @@ class MigrationLoader(object): @classmethod def migrations_module(cls, app_label): - if settings.MIGRATION_MODULES.get(app_label): + if app_label in settings.MIGRATION_MODULES: return settings.MIGRATION_MODULES[app_label] else: app_package_name = apps.get_app_config(app_label).name diff --git a/django/utils/datastructures.py b/django/utils/datastructures.py index ad8792d192d..2efcd00eb06 100644 --- a/django/utils/datastructures.py +++ b/django/utils/datastructures.py @@ -244,26 +244,6 @@ class SortedDict(dict): self.keyOrder = [] -def dict_merge(a, b): - """ - Utility to recursively merge two dicts, taking care not to overwrite subkeys - (which would happen with dict.update), but keeping existing key including - those from subdictionaries (optionally opted-out if a `_clear_defaults` key - is present). - Thanks Ross McFarland (https://www.xormedia.com/recursively-merge-dictionaries-in-python/) - """ - if b.get('_clear_defaults'): - return copy.deepcopy(b) - - result = copy.deepcopy(a) - for key, value in six.iteritems(b): - if key in a and isinstance(result[key], dict): - result[key] = dict_merge(result[key], value) - else: - result[key] = value - return result - - class OrderedSet(object): """ A set which keeps the ordering of the inserted items. diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index f38d0990b23..209e967865b 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -533,9 +533,6 @@ Miscellaneous widget to allow more customization. The undocumented ``url_markup_template`` attribute was removed in favor of ``template_with_initial``. -* When a dictionary setting is overridden in user settings, both dictionaries - are merged by default. See :ref:`dictionary-settings`. - * For consistency with other major vendors, the ``en_GB`` locale now has Monday as the first day of the week. diff --git a/docs/topics/settings.txt b/docs/topics/settings.txt index cc9cc21652a..2024d88a0f7 100644 --- a/docs/topics/settings.txt +++ b/docs/topics/settings.txt @@ -110,32 +110,6 @@ between the current settings file and Django's default settings. For more, see the :djadmin:`diffsettings` documentation. -.. _dictionary-settings: - -Overriding dictionary settings ------------------------------- - -.. versionchanged:: 1.8 - -When defining a dictionary-type setting which has a non-empty value (see -:setting:`CACHES` for example), you do not have to redefine all its keys. You -can just define the keys differing from the default, and Django will simply -merge your setting value with the default value. For example, if you define -:setting:`CACHES` so:: - - CACHES = { - 'special': { - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', - 'LOCATION': '127.0.0.1:11211', - } - } - -then ``CACHES['default']`` which is set by default in Django's global settings -will still be defined, as well as the new ``'special'`` cache backend. - -If you want your setting to completely override the default value, you can add -a ``_clear_defaults`` key with a ``True`` value to the dictionary. - Using settings in Python code ============================= diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 70a5e1165d2..4677791fcdc 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -522,7 +522,6 @@ class BaseCacheTests(object): def _perform_cull_test(self, cull_cache, initial_count, final_count): # Create initial cache key entries. This will overflow the cache, # causing a cull. - cull_cache.clear() for i in range(1, initial_count): cull_cache.set('cull%d' % i, 'value', 1000) count = 0 @@ -919,10 +918,7 @@ class DBCacheTests(BaseCacheTests, TransactionTestCase): stdout=stdout ) self.assertEqual(stdout.getvalue(), - "Cache table 'test cache table' already exists.\n" * len([ - k for k, v in settings.CACHES.items() - if v['BACKEND'] == 'django.core.cache.backends.db.DatabaseCache']) - ) + "Cache table 'test cache table' already exists.\n" * len(settings.CACHES)) def test_createcachetable_with_table_argument(self): """ diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py index 7563381abee..d07eecfcef1 100644 --- a/tests/migrations/test_executor.py +++ b/tests/migrations/test_executor.py @@ -196,7 +196,7 @@ class ExecutorTests(MigrationTestBase): @override_settings( MIGRATION_MODULES={ "migrations": "migrations.test_migrations_custom_user", - "auth": "django.contrib.auth.migrations", + "django.contrib.auth": "django.contrib.auth.migrations", }, AUTH_USER_MODEL="migrations.Author", ) diff --git a/tests/migrations/test_loader.py b/tests/migrations/test_loader.py index 9f7ce9a8e7b..68d05d296f1 100644 --- a/tests/migrations/test_loader.py +++ b/tests/migrations/test_loader.py @@ -81,10 +81,7 @@ class LoaderTests(TestCase): # Ensure we've included unmigrated apps in there too self.assertIn("basic", project_state.real_apps) - @override_settings(MIGRATION_MODULES={ - "_clear_defaults": True, - "migrations": "migrations.test_migrations_unmigdep" - }) + @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_unmigdep"}) def test_load_unmigrated_dependency(self): """ Makes sure the loader can load migrations with a dependency on an unmigrated app. diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py index 784346946d0..2dcf62abb5f 100644 --- a/tests/settings_tests/tests.py +++ b/tests/settings_tests/tests.py @@ -273,63 +273,6 @@ class SettingsTests(TestCase): self.assertRaises(ValueError, setattr, settings, 'ALLOWED_INCLUDE_ROOTS', '/var/www/ssi/') - def test_dict_setting(self): - """ - Test that dictionary-type settings can be "complemented", that is existing - setting keys/values are not overriden by user settings, but merged into the - existing dict. - """ - s = LazySettings() # Start with fresh settings from global_settings.py - # Simply overwriting the key - s.configure(CACHES={'default': {'BACKEND': 'django.core.cache.backends.dummy.DummyCache'}}) - self.assertEqual(s.CACHES['default']['BACKEND'], - 'django.core.cache.backends.dummy.DummyCache') - - s = LazySettings() - # More complex overwriting - s.configure(CACHES={ - 'default': {'LOCATION': 'unique-snowflake'}, - 'temp': {'BACKEND': 'django.core.cache.backends.dummy.DummyCache'} - }) - self.assertDictEqual(s.CACHES, { - 'default': { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'unique-snowflake' - }, - 'temp': { - 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', - } - }) - - def test_dict_setting_clear_defaults(self): - """ - Test the ability to deactivate the merge feature of dictionary settings. - """ - s = LazySettings() - s.configure(CACHES={ - '_clear_defaults': True, - 'temp': {'BACKEND': 'django.core.cache.backends.dummy.DummyCache'} - }) - self.assertDictEqual(s.CACHES, { - '_clear_defaults': True, - 'temp': {'BACKEND': 'django.core.cache.backends.dummy.DummyCache'} - }) - - # Also work on a subkey - s = LazySettings() - s.configure(CACHES={ - 'default': { - '_clear_defaults': True, - 'LOCATION': 'unique-snowflake', - } - }) - self.assertDictEqual(s.CACHES, { - 'default': { - '_clear_defaults': True, - 'LOCATION': 'unique-snowflake', - } - }) - class TestComplexSettingOverride(TestCase): def setUp(self):