This change has been marked as completed.

Describe the completed change (optional):

Pushed to master (a5fdabc)

Summary

Provide a nicer selector for cache backends compatible with settings.CACHES.

Review Request #3603 — Created Dec. 1, 2012 and submitted 12 years, 7 months ago — Latest diff uploaded 12 years, 7 months ago

Information

Owner

chipx86*

Repository

Review Board

Branch

release-1.7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description*

Provide a nicer selector for cache backends compatible with settings.CACHES.

The cache backend selector in General Settings is now much nicer,
providing a dropdown of possible cache types and an appropriate field
for entering the hosts or cache location.

Two entries are provided by default: Memcached and File cache.

If running with DEBUG = True, then "Local memory cache" will also be
available, making it easier to test and debug things.

If some other cache backend is used, then "Custom" will be an option as
well, and will leave the cache settings untouched.

This moves us away fully from the old Django CACHE_BACKEND setting.

Testing Done

Tested each cache backend and verified that the correct cache settings
were stored in siteconfig and were being loaded out correctly.

Files

File cache fields

memcached fields

Diff Revision 2 (Latest)

orig

-    Revision ea8edd60ed8f273f46af0da4a5cc6012c8b0abc2
+    New Change
     import re
     import urlparse
     from django import forms
     from django.contrib.sites.models import Site
-    from django.core.cache import parse_backend_uri, InvalidCacheBackendError
+    from django.conf import settings
     from django.core.cache import parse_backend_uri, InvalidCacheBackendError, \
                                   DEFAULT_CACHE_ALIAS
     from django.utils.translation import ugettext as _
     from djblets.log import restart_logging
     from djblets.siteconfig.forms import SiteSettingsForm
     from djblets.util.cache import normalize_cache_backend
     from djblets.util.forms import TimeZoneField
     from reviewboard.accounts.forms import LegacyAuthModuleSettingsForm
     from reviewboard.admin.checks import get_can_enable_search, \
                                          get_can_enable_syntax_highlighting, \
                                          get_can_use_amazon_s3, \
                                          get_can_use_couchdb
     from reviewboard.admin.siteconfig import load_site_config
     from reviewboard.ssh.client import SSHClient
     class GeneralSettingsForm(SiteSettingsForm):
         """General settings for Review Board."""
         CACHE_TYPE_CHOICES = (
             ('memcached', _('Memcached')),
             ('file', _('File cache')),
+        )
         CACHE_BACKENDS_MAP = {
             'file': 'django.core.cache.backends.filebased.FileBasedCache',
             'memcached': 'django.core.cache.backends.memcached.CacheClass',
             'locmem': 'django.core.cache.backends.locmem.LocMemCache',
+        }
         CACHE_TYPES_MAP = {
             'django.core.cache.backends.filebased.FileBasedCache': 'file',
             'django.core.cache.backends.memcached.CacheClass': 'memcached',
             'django.core.cache.backends.locmem.LocMemCache': 'locmem',
+        }
         CACHE_LOCATION_FIELD_MAP = {
             'file': 'cache_path',
             'memcached': 'cache_host',
+        }
         server = forms.CharField(
             label=_("Server"),
             help_text=_("The URL of this Review Board server. This should not "
                         "contain the subdirectory Review Board is installed in."),
             widget=forms.TextInput(attrs={'size': '30'}))
             help_text=_("The directory that search index data should be stored "
                         "in."),
             required=False,
             widget=forms.TextInput(attrs={'size': '50'}))
-        cache_backend = forms.CharField(
+        cache_type = forms.ChoiceField(
             label=_("Cache Backend"),
-            help_text=_("The path to the cache backend."
+            choices=CACHE_TYPE_CHOICES,
-                        "Example: 'memcached://127.0.0.1:11211/'"),
+            help_text=_('The type of server-side caching to use.'),
-            required=False,
+            required=True)
         cache_path = forms.CharField(
             label=_("Cache Path"),
             help_text=_('The file location for the cache.'),
             required=True,
             widget=forms.TextInput(attrs={'size': '50'}))
         cache_host = forms.CharField(
             label=_("Cache Hosts"),
             help_text=_('The host or hosts used for the cache, in hostname:port '
                         'form. Multiple hosts can be specified by separating '
                         'them with a semicolon (;).'),
             required=True,
             widget=forms.TextInput(attrs={'size': '50'}))
         def load(self):
             domain_method = self.siteconfig.get("site_domain_method")
             site = Site.objects.get_current()
             can_enable_search, reason = get_can_enable_search()
             if not can_enable_search:
                 self.disabled_fields['search_enable'] = True
                 self.disabled_fields['search_index_file'] = True
                 self.disabled_reasons['search_enable'] = reason
             # Load the rest of the settings from the form.
             super(GeneralSettingsForm, self).load()
             # Load the cache settings.
             cache_backend = normalize_cache_backend(
                 self.siteconfig.get('cache_backend'))
             cache_type = self.CACHE_TYPES_MAP.get(cache_backend['BACKEND'],
                                                   'custom')
             self.fields['cache_type'].initial = cache_type
             if settings.DEBUG:
                 self.fields['cache_type'].choices += (
                     ('locmem', _('Local memory cache')),
+                )
             if cache_type == 'custom':
                 self.fields['cache_type'].choices += (
                     ('custom', _('Custom')),
+                )
                 cache_locations = []
             elif cache_type != 'locmem':
                 cache_locations = cache_backend['LOCATION']
                 if not isinstance(cache_locations, list):
                     cache_locations = [cache_locations]
                 location_field = self.CACHE_LOCATION_FIELD_MAP[cache_type]
                 self.fields[location_field].initial = ';'.join(cache_locations)
             # This must come after we've loaded the general settings.
             self.fields['server'].initial = "%s://%s" % (domain_method,
                                                          site.domain)
         def save(self):
             site.domain = domain_name
             site.save()
             self.siteconfig.set("site_domain_method", domain_method)
             cache_type = self.cleaned_data['cache_type']
             if cache_type != 'custom':
                 if cache_type == 'locmem':
                     # We want to specify a "reviewboard" location to keep items
                     # separate from those in other caches.
                     location = 'reviewboard'
                 else:
                     location_field = self.CACHE_LOCATION_FIELD_MAP[cache_type]
                     location = self.cleaned_data[location_field]
                     if cache_type == 'memcached':
                         # memcached allows a list of servers, rather than just a
                         # string representing one.
                         location = location.split(';')
                 self.siteconfig.set('cache_backend', {
                     DEFAULT_CACHE_ALIAS: {
                         'BACKEND': self.CACHE_BACKENDS_MAP[cache_type],
                         'LOCATION': location,
+                    }
                 })
             super(GeneralSettingsForm, self).save()
             # Reload any important changes into the Django settings.
             load_site_config()
-        def clean_cache_backend(self):
+        def full_clean(self):
-            """Validates that the specified cache backend is parseable by Django."""
+            cache_type = self['cache_type'].data or self['cache_type'].initial
             backend = self.cleaned_data['cache_backend'].strip()
-            if backend:
+            for iter_cache_type, field in self.CACHE_LOCATION_FIELD_MAP.iteritems():
-                try:
+                self.fields[field].required = (cache_type == iter_cache_type)
                     parse_backend_uri(backend)
-                except InvalidCacheBackendError, e:
+            return super(GeneralSettingsForm, self).full_clean()
-                    raise forms.ValidationError(e)
-            return backend
+        def clean_cache_host(self):
             cache_host = self.cleaned_data['cache_host'].strip()
             if self.fields['cache_host'].required and not cache_host:
                 raise forms.ValidationError(
                     _('A valid cache host must be provided.'))
             return cache_host
         def clean_cache_path(self):
             cache_path = self.cleaned_data['cache_path'].strip()
             if self.fields['cache_path'].required and not cache_path:
                 raise forms.ValidationError(
                     _('A valid cache path must be provided.'))
             return cache_path
         def clean_search_index_file(self):
             """Validates that the specified index file is valid."""
             index_file = self.cleaned_data['search_index_file'].strip()
             return index_file
         class Meta:
             title = _("General Settings")
-            save_blacklist = ('server',)
+            save_blacklist = ('server', 'cache_type', 'cache_host', 'cache_path')
             fieldsets = (
+                {
                     'classes': ('wide',),
                     'title':   _("Site Settings"),
                     'fields':  ('server', 'site_media_url',
                                 'site_admin_name',
                                 'site_admin_email',
-                                'locale_timezone',
+                                'locale_timezone'),
-                                'cache_backend'),
+                },
+                {
                     'classes': ('wide',),
                     'title': _('Cache Settings'),
                     'fields': ('cache_type', 'cache_path', 'cache_host'),
                 },
+                {
                     'classes': ('wide',),
                     'title':   _("Search"),
                     'fields':  ('search_enable', 'search_index_file'),

reviewboard/admin/urls.py

reviewboard/templates/admin/general_settings.html

	reviewboard/admin/forms.py
	reviewboard/admin/urls.py
	reviewboard/templates/admin/general_settings.html