ecometrica / django-multisite

Multisite in django — use one Django app to serve multiple domains
BSD 3-Clause "New" or "Revised" License
138 stars 42 forks source link

Incorrect cache prefix documentation #43

Closed kousu closed 7 years ago

kousu commented 7 years ago

Our docs claim that CACHE_MULTISITE_KEY_PREFIX = "" is its default value, but if you actually set that in your settings file this test fails:

======================================================================
FAIL: test_default_key_prefix (multisite.tests.SiteCacheTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/nguenthe/django-multisite/.venv-python2/lib/python2.7/site-packages/django/test/utils.py", line 181, in inner
    return test_func(*args, **kwargs)
  File "/Users/nguenthe/django-multisite/multisite/tests.py", line 447, in test_default_key_prefix
    settings.CACHES['default']['KEY_PREFIX'], self.site.id
AssertionError: u'sites..2' != u'sites.test1.2'
- sites..2
+ sites.test1.2
?       +++++

The code that handles this is tangly and terrible and is literally in a file called "hacks.py":

        if cache is None:
            cache_alias = getattr(settings, 'CACHE_MULTISITE_ALIAS', 'default')
            self._key_prefix = getattr(
                settings,
                'CACHE_MULTISITE_KEY_PREFIX',
                settings.CACHES[cache_alias].get('KEY_PREFIX', '')
            )
            cache = get_cache(cache_alias)
            self._warn_cache_backend(cache, cache_alias)
        else:
            self._key_prefix = getattr(
                settings, 'CACHE_MULTISITE_KEY_PREFIX', cache.key_prefix
            )

It's not clear to me who is wrong here: hacks.py or the documentation of what it's supposed to do. Does anyone have any experience with CACHE_MULTISITE_KEY_PREFIX? What are your thoughts? If there's not much enthusiasm for it I intend to remove it to curtail the convoluted configuration space.

kousu commented 7 years ago

Upon a moment's reflection, I suppose that the intent here was for an falseish or missing value to trigger the fallback; it's a mistake that the simple existence of the value should be trusted. So that way the default value would do something sensible.

rebkwok commented 7 years ago

Fixed in PR #45