Merge lp:~bloodearnest/django-preflight/gargoyle2 into lp:django-preflight

[Simon Davy]

Add support for gargoyle flags in preflight

Add support for gargoyle flags in preflight

[Simon Davy (bloodearnest)]

Continuation of mfoords MP: https://code.launchpad.net/~canonical-isd-hackers/django-preflight/gargoyle/+merge/106374

[Michael Foord (mfoord)]

The code should not depend on settings.USE_GARGOYLE.

Other than that all looks good.

review: Needs Fixing

[Ricardo Kirkner (ricardokirkner)]

- swap l. 72 and 73 around
- l 109, 120, 143, 155: fix whitespace before :
- l.280: why not add in l.270?
- l.398: what's the need of renaming the example_project folder?
- l.428: since you renamed the db, why add the old name to bzrignore? (l. 9)

[Ricardo Kirkner (ricardokirkner)]

Can you also include a screenshot of how the new template looks like with the flags?

[Simon Davy (bloodearnest)]

> The code should not depend on settings.USE_GARGOYLE.
>
> Other than that all looks good.

Done

[Simon Davy (bloodearnest)]

> - swap l. 72 and 73 around
> - l 109, 120, 143, 155: fix whitespace before :
> - l.280: why not add in l.270?

Done the above

> - l.398: what's the need of renaming the example_project folder?

This is unfortunately necessary because the nexus package (a dep of gargoyle) is higher in sys.pth when run with "python setup.py test", and it erroneously includes its example_project directory, meaning that the test_suite in setup.py ("example_project.run.tests") was finding the wrong example_project module and failing.

Pull request to exclude example_project upstream: https://github.com/disqus/nexus/pull/5

> - l.428: since you renamed the db, why add the old name to bzrignore? (l. 9)

Done - a left over from earlier

[lp:~bloodearnest/django-preflight/gargoyle2]

26. By Simon Davy on 2013-06-03

addressing review comments

[Simon Davy (bloodearnest)]

Screen shot

http://ubuntuone.com/7Khn6iSg9KAPF6JDCVebk0

[Michael Foord (mfoord)]

Good work. Thanks.

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2012-05-02 21:37:14 +0000
+++ .bzrignore	2013-06-03 10:45:34 +0000
@@ -4,3 +4,4 @@
 .tox
 *.egg-info
 doc/_build
+preflight.wp*

=== modified file 'doc/conf.py'
--- doc/conf.py	2011-02-09 16:26:04 +0000
+++ doc/conf.py	2013-06-03 10:45:34 +0000
@@ -18,7 +18,7 @@
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 sys.path.append(os.path.abspath('..'))
 
-os.environ['DJANGO_SETTINGS_MODULE'] = 'example_project.settings'
+os.environ['DJANGO_SETTINGS_MODULE'] = 'preflight_example_project.settings'
 
 # -- General configuration -----------------------------------------------------
 

=== modified file 'doc/install.rst'
--- doc/install.rst	2013-05-17 14:52:42 +0000
+++ doc/install.rst	2013-06-03 10:45:34 +0000
@@ -56,10 +56,10 @@
 
     import preflight
     import preflight.urls
-    
-    
+
+
     preflight.autodiscover()
-    
+
     urlpatterns = patterns('',
         (r'^preflight/', include(preflight.urls)),
     )

=== modified file 'preflight/models.py'
--- preflight/models.py	2013-02-20 21:15:50 +0000
+++ preflight/models.py	2013-06-03 10:45:34 +0000
@@ -142,3 +142,39 @@
     To be able to access you need to have permission from every preflight class.
     """
     return all(class_().authenticate(request) for class_ in REGISTRY)
+
+
+def gather_switches():
+    """Gather all switches into one list."""
+    switches = {}
+    gargoyle_switches = gather_gargoyle()
+    if gargoyle_switches:
+        switches['gargoyle'] = gargoyle_switches
+    return switches
+
+
+def gather_gargoyle():
+    try:
+        from gargoyle.models import Switch
+    except ImportError:
+        return None
+    return [_format_gargoyle(switch) for switch in Switch.objects.all()]
+
+
+def _format_gargoyle(switch):
+    """Formats a gargoyle switch into std format for display"""
+    from gargoyle import gargoyle
+    d = switch.to_dict(gargoyle)
+    conditions = []
+    for condition in d['conditions']:
+        params = ','.join('%s=%s' % (c[0], c[2])
+                          for c in condition['conditions'])
+        conditions.append("%s(%s)" % (condition['label'], params))
+
+    return dict(
+        name=d['key'],
+        description=d['description'],
+        status=d['status'],
+        status_text=d['statusLabel'],
+        conditions=conditions,
+    )

=== modified file 'preflight/templates/preflight/overview.html'
--- preflight/templates/preflight/overview.html	2012-05-02 21:37:14 +0000
+++ preflight/templates/preflight/overview.html	2013-06-03 10:45:34 +0000
@@ -11,8 +11,21 @@
 <div id="preflight">
   <h1>{% trans "Pre-flight Applications Checks" %}</h1>
 
+  <ul id="links">
+    {% for application in applications %}
+      <li><a href="#{{ application.name|slugify }}">{{ application.name }}</a></li>
+    {% endfor %}
+    <li><a href="#versions">Versions</a></li>
+    <li><a href="#settings">Settings</a></li>
+    {% if switches %}
+    <li><a href="#switches">Switches</a></li>
+    {% endif %}
+  </ul>
+
   {% for application in applications %}
-  <h2>{{ application.name }}</h2>
+  <a href="#links" style="text-decoration: none;">
+    <h2 id="{{ application.name|slugify }}">{{ application.name }}</h2>
+  </a>
   <table class="{{ preflight_table_class }}">
     <thead>
       <tr>
@@ -39,7 +52,9 @@
   </table>
   {% endfor %}
 
-  <h1>{% trans "Versions" %}</h1>
+  <a href="#links" style="text-decoration: none;">
+    <h1 id="versions">{% trans "Versions" %}</h1>
+  </a>
   <table class="{{ preflight_table_class }}">
     <thead>
       <tr>
@@ -49,15 +64,17 @@
     </thead>
     <tbody>
       {% for item in versions %}
-      <tr>
-        <td>{{ item.name }}</td>
-        <td>{{ item.version }}</td>
-      </tr>
+        <tr>
+          <td>{{ item.name }}</td>
+          <td>{{ item.version }}</td>
+        </tr>
       {% endfor %}
     </tbody>
   </table>
 
-  <h1>{% trans "Settings" %}</h1>
+  <a href="#links" style="text-decoration: none;">
+    <h1 id="settings">{% trans "Settings" %}</h1>
+  </a>
   <dl class="{{ preflight_table_class }}">
   {% for setting in settings %}
     <dt><b>{{ setting.name }}</b></dt>
@@ -65,6 +82,50 @@
     {% if setting.location %}<dd>Location: {{ setting.location}}</dd>{% endif %}
   {% endfor %}
   </dl>
+
+  {% if switches %}
+  <a href="#links" style="text-decoration: none;">
+    <h1 id="switches">{% trans "Switches" %}</h1>
+  </a>
+    <table id="switches-table" class="{{ preflight_table_class }}">
+      <thead>
+        <tr>
+          <th>{% trans "Name" %}</th>
+          <th>{% trans "Description" %}</th>
+          <th>{% trans "Conditions" %}</th>
+          <th>{% trans "Status" %}</th>
+        </tr>
+      </thead>
+      <tbody>
+        {% for type, type_switches in switches.items %}
+        <tr><th colspan="4">{{ type }}</th></tr>
+        {% for switch in type_switches %}
+        <tr class="switch">
+            <td>{{ switch.name }}</td>
+            <td>{{ switch.description }}</td>
+            <td>
+                {% if switch.conditions %}
+                <ul>
+                    {% for condition in switch.conditions  %}
+                    <li>{{ condition|first }}: {{ condition|last }}</li>
+                    {% endfor %}
+                </ul>
+                {% endif %}
+            </td>
+            <td>{{ switch.status_text }}</td>
+        {% endfor %}
+        </tr>
+        {% endfor %}
+      </tbody>
+  </table>
+  <p>Switch data is included in JSON format in a script tag with id
+      "switches-json" to aid automated scripting</p>
+  <script id="switches-json">
+    {{ switches_json|safe }}
+  </script>
+  {% else %}
+    <p>No switches.</p>
+  {% endif %}
 </div>
 
 <p>View generated at: {{ now }}</p>

=== modified file 'preflight/tests.py'
--- preflight/tests.py	2013-05-17 14:51:54 +0000
+++ preflight/tests.py	2013-06-03 10:45:34 +0000
@@ -1,18 +1,26 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 import sys
+import json
+try:
+    from unittest import skipIf
+except ImportError:
+    from unittest2 import skipIf  # NOQA
 
+from django.conf import settings
 from django.core.management import call_command
 from django.core.urlresolvers import reverse
 from django.http import HttpResponse
 from django.test import TestCase
-from django.template import RequestContext, TemplateDoesNotExist
+from django.template import RequestContext
+from django.template.loader import render_to_string
 from mock import (
     Mock,
     patch,
 )
 
 from cStringIO import StringIO
+from pyquery import PyQuery
 
 from . import Preflight
 from .models import (
@@ -21,9 +29,12 @@
     REGISTRY,
     authenticate,
     gather_checks,
+    gather_switches,
+    gather_gargoyle,
     gather_settings,
     gather_versions,
 )
+from preflight.conf import BASE_TEMPLATE, TABLE_CLASS
 
 
 class CheckTestCase(TestCase):
@@ -142,6 +153,7 @@
     def authenticate(self, request):
         return False
 
+
 def clear_registry():
     while REGISTRY:
         REGISTRY.pop()
@@ -182,6 +194,7 @@
     def versions(self):
         return [{'name': 'spam', 'version': 'ni'}]
 
+
 class ExtraVersionAsList(Preflight):
 
     versions = [{'name': 'eggs', 'version': 'peng'}]
@@ -294,3 +307,100 @@
         mock_settings.PREFLIGHT_HIDDEN_SETTINGS = ''
         settings = gather_settings()
         self.assertEqual([], settings)
+
+
+@skipIf(not settings.USE_GARGOYLE, 'skipping for Django 1.1')
+class GargoyleTestCase(TestCase):
+    from gargoyle import gargoyle  # NOQA
+    from gargoyle.builtins import IPAddressConditionSet # NOQA
+    from gargoyle.models import (  # NOQA
+        Switch,
+        DISABLED,
+        SELECTIVE,
+        GLOBAL,
+        INHERIT
+    )
+
+    def setUp(self):
+        super(GargoyleTestCase, self).setUp()
+        self.gargoyle.register(self.IPAddressConditionSet())
+
+    def get_switches(self):
+        switches = [
+            self.Switch(key='DISABLED', status=self.DISABLED,
+                description='switch 1'),
+            self.Switch(key='SELECTIVE_1', status=self.SELECTIVE),
+            self.Switch(key='SELECTIVE_2', status=self.SELECTIVE),
+            self.Switch(key='GLOBAL', status=self.GLOBAL),
+            self.Switch(key='INHERIT', status=self.INHERIT),
+        ]
+        selective = switches[2]
+        selective.add_condition(
+            self.gargoyle,
+            condition_set='gargoyle.builtins.IPAddressConditionSet',
+            field_name='ip_address',
+            condition='127.0.0.1',
+        )
+        return switches
+
+    @patch.dict(sys.modules, **{'gargoyle.models': None})
+    def test_gather_switches_no_gargoyle(self):
+        self.assertEqual(gather_gargoyle(), None)
+
+    def assert_switches_dict(self, actual):
+        expected = [
+            dict(name='DISABLED',
+                 status=self.DISABLED,
+                 description='switch 1',
+                 status_text=self.Switch.STATUS_LABELS[self.DISABLED],
+                 conditions=[]),
+            dict(name='SELECTIVE_1',
+                 status=self.SELECTIVE,
+                 description=None,
+                 status_text=self.Switch.STATUS_LABELS[self.GLOBAL],
+                 conditions=[]),
+            dict(name='SELECTIVE_2', status=self.SELECTIVE,
+                 description=None,
+                 status_text=self.Switch.STATUS_LABELS[self.SELECTIVE],
+                 conditions=['IP Address(ip_address=127.0.0.1)']),
+            dict(name='GLOBAL', status=self.GLOBAL,
+                 description=None,
+                 status_text=self.Switch.STATUS_LABELS[self.GLOBAL],
+                 conditions=[]),
+            dict(name='INHERIT', status=self.INHERIT,
+                 description=None,
+                 status_text=self.Switch.STATUS_LABELS[self.INHERIT],
+                 conditions=[]),
+        ]
+        self.assertEqual(actual, expected)
+
+    @patch('gargoyle.models.Switch.objects.all')
+    def test_gather_switches(self, mock_all):
+        mock_all.return_value = self.get_switches()
+        self.assert_switches_dict(gather_gargoyle())
+
+    @patch('gargoyle.models.Switch.objects.all')
+    def test_gargoyle_template(self, mock_all):
+        switches = self.get_switches()
+        mock_all.return_value = switches
+        the_switches = gather_switches()
+        context = {
+            "switches": the_switches,
+            "switches_json": json.dumps(the_switches),
+            "preflight_base_template": BASE_TEMPLATE,
+            "preflight_table_class": TABLE_CLASS,
+        }
+        response = render_to_string('preflight/overview.html', context)
+        dom = PyQuery(response)
+        table = dom.find('#switches-table tbody')
+        self.assertEqual(table.find('tr')[0][0].text, 'gargoyle')
+
+        for row, switch in zip(table.find('tr.switch'), switches):
+            self.assertEqual(row[0].text, switch.key)
+            self.assertEqual(row[1].text, str(switch.description))
+            self.assertEqual(row[3].text, switch.get_status_label())
+
+        data = json.loads(dom.find('#switches-json').text())
+        self.assertTrue('gargoyle' in data)
+        json_switches = data['gargoyle']
+        self.assert_switches_dict(json_switches)

=== modified file 'preflight/views.py'
--- preflight/views.py	2013-05-17 14:51:29 +0000
+++ preflight/views.py	2013-06-03 10:45:34 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from datetime import datetime
+import json
 
 from django.views.decorators.cache import never_cache
 from django.http import Http404
@@ -13,6 +14,7 @@
     gather_checks,
     gather_settings,
     gather_versions,
+    gather_switches,
 )
 from .conf import BASE_TEMPLATE, TABLE_CLASS
 
@@ -22,10 +24,13 @@
     if not authenticate(request):
         raise Http404
 
+    switches = gather_switches()
     context = RequestContext(request, {
         "applications": gather_checks(),
         "versions": gather_versions(),
         "settings": gather_settings(),
+        "switches": switches,
+        "switches_json": json.dumps(switches),
         "now": datetime.now(),
         "preflight_base_template": BASE_TEMPLATE,
         "preflight_table_class": TABLE_CLASS,

=== renamed directory 'example_project' => 'preflight_example_project'
=== modified file 'preflight_example_project/run.py'
--- example_project/run.py	2011-02-07 16:06:51 +0000
+++ preflight_example_project/run.py	2013-06-03 10:45:34 +0000
@@ -1,8 +1,8 @@
 import os
 import sys
 
-os.environ['DJANGO_SETTINGS_MODULE'] = 'example_project.settings'
-sys.path.insert(0, 'example_project')
+os.environ['DJANGO_SETTINGS_MODULE'] = 'preflight_example_project.settings'
+sys.path.insert(0, 'preflight_example_project')
 
 try:
     from django.test.utils import get_runner

=== modified file 'preflight_example_project/settings.py'
--- example_project/settings.py	2013-05-17 14:51:54 +0000
+++ preflight_example_project/settings.py	2013-06-03 10:45:34 +0000
@@ -1,4 +1,5 @@
 import os
+import django
 
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
@@ -9,7 +10,7 @@
 DATABASES = {
     'default': {
         'ENGINE': 'django.db.backends.sqlite3',
-        'NAME': 'database.sqlit3',
+        'NAME': 'database.sqlite3',
     }
 }
 
@@ -25,7 +26,7 @@
 ADMIN_MEDIA_PREFIX = '/media/'
 SECRET_KEY = 'j0-wo!y4zwi)tejv1u(0skc41_379ls^@qbh91%#5o=greje6('
 
-ROOT_URLCONF = 'example_project.urls'
+ROOT_URLCONF = 'preflight_example_project.urls'
 
 TEMPLATE_DIRS = (
     os.path.join(os.path.realpath(os.path.dirname(__file__)), 'templates'),
@@ -52,4 +53,8 @@
     'app',
 )
 
+USE_GARGOYLE = django.VERSION[:2] > (1, 1)
+if USE_GARGOYLE:
+    INSTALLED_APPS = INSTALLED_APPS + ('gargoyle',)
+
 PREFLIGHT_BASE_TEMPLATE = 'base.html'

=== modified file 'setup.py'
--- setup.py	2013-05-17 14:55:49 +0000
+++ setup.py	2013-06-03 10:45:34 +0000
@@ -1,6 +1,8 @@
 # -*- encoding: utf-8 -*-
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+import sys
+
 try:
     from ast import PyCF_ONLY_AST
 except ImportError:
@@ -14,6 +16,14 @@
             for line in open('preflight/__init__.py')
             if line.startswith('__version__')][0]
 
+tests_require = [
+    'mock > 0.6',
+    'gargoyle >= 0.6.0',
+    'pyquery',
+]
+
+if sys.version_info[:2] < (2, 7):
+    tests_require.append('unittest2')
 
 setup(
     name='django-preflight',
@@ -46,13 +56,11 @@
         'preflight': ['templates/preflight/*.html'],
     },
     install_requires=[
-        'django >= 1.1'
-    ],
-    tests_require=[
-        'mock > 0.6'
-    ],
+        'django >= 1.1',
+    ],
+    tests_require=tests_require,
     extras_require={
         'docs': ['Sphinx'],
     },
-    test_suite='example_project.run.tests',
+    test_suite='preflight_example_project.run.tests',
 )