Merge lp:~thumper/charms/precise/python-django/tweaks into lp:charms/python-django

[Tim Penhey]

Extract out a method for the database sync calls.

This means that when the charm is updated to support Django 1.7, there is only one place that needs to deal with it. The existing behaviour has been kept, even though it is a little weird in one place.

Also the injection templates for urls and settings is updated to make the execution call a little more helpful if there are errors as filenames are shown along-side the code.

[Charles Butler (lazypower)]

my cursory glance on this is good, but I cannot verify this didn't produce test weirdness. Waiting on the automated testing infra to kick this off and give us some results; as well as let Patrick have some time to look this over as he just landed the pure-python branch earlier today.

Thanks for the submission Thumper, awesome to see you're submitting charm patches now.

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-10-09 19:55:08 +0000
+++ hooks/hooks.py	2015-01-08 00:39:19 +0000
@@ -238,6 +238,33 @@
             inject_file.write(str(template))
 
 
+def update_database_schema(exit_on_error=True, swallow_exceptions=False):
+    """Run the appropriate syncdb/migrate calls.
+
+    This will make sure that the database reflects the currently installed
+    applications.
+    """
+    django_admin_cmd = find_django_admin_cmd()
+    # TODO: only run syncdb if Django version is < 1.7
+    try:
+        run("%s syncdb --noinput --settings=%s" %
+            (django_admin_cmd, settings_module),
+            exit_on_error=exit_on_error, cwd=working_dir)
+    except:
+        if not swallow_exceptions:
+            raise
+
+    # TODO: check should be (django_south or Django version > 1.7)
+    if django_south:
+        try:
+            run("%s migrate --settings=%s" %
+                (django_admin_cmd, settings_module),
+                exit_on_error=exit_on_error, cwd=working_dir)
+        except:
+            if not swallow_exceptions:
+                raise
+
+
 ###############################################################################
 # Hook functions
 ###############################################################################
@@ -413,20 +440,8 @@
 
     run('chown -R %s:%s %s' % (wsgi_user, wsgi_group, vcs_clone_dir))
 
-    try:
-        run("%s syncdb --noinput --settings=%s" %
-            (django_admin_cmd, settings_module),
-            exit_on_error=False, cwd=working_dir)
-    except:
-        pass
-
-    if django_south:
-        try:
-            run("%s migrate --settings=%s" %
-                (django_admin_cmd, settings_module),
-                exit_on_error=False, cwd=working_dir)
-        except:
-            pass
+    # It isn't entirely clear to me why we don't want to exit on an error here.
+    update_database_schema(exit_on_error=False, swallow_exceptions=True)
 
     # Trigger WSGI reloading
     for relid in relation_ids('wsgi'):
@@ -441,7 +456,6 @@
 @hooks.hook('pgsql-relation-joined', 'pgsql-relation-changed')
 def pgsql_relation_joined_changed():
     os.environ['DJANGO_SETTINGS_MODULE'] = settings_module
-    django_admin_cmd = find_django_admin_cmd()
 
     packages = ["python-psycopg2", "postgresql-client"]
     apt_install(packages, options=['--force-yes'])
@@ -461,17 +475,11 @@
     process_template('pgsql_engine.tmpl', templ_vars,
                      settings_database_path % {'engine_name': 'pgsql'})
 
-    run("%s syncdb --noinput --settings=%s" %
-        (django_admin_cmd, settings_module),
-        cwd=working_dir)
-
     if django_south:
         south_config_file = os.path.join(settings_dir_path, '50-south.py')
         process_template('south.tmpl', {}, south_config_file)
 
-        run("%s migrate --settings=%s" %
-            (django_admin_cmd, settings_module),
-            cwd=working_dir)
+    update_database_schema()
 
     run('chown -R %s:%s %s' % (wsgi_user, wsgi_group, vcs_clone_dir))
 
@@ -494,7 +502,6 @@
             'mysql-shared-relation-joined', 'mysql-shared-relation-changed')
 def mysql_relation_joined_changed():
     os.environ['DJANGO_SETTINGS_MODULE'] = settings_module
-    django_admin_cmd = find_django_admin_cmd()
 
     packages = ["python-mysqldb", "mysql-client"]
     apt_install(packages, options=['--force-yes'])
@@ -513,17 +520,11 @@
 
     process_template('mysql_engine.tmpl', templ_vars, settings_database_path % {'engine_name': 'mysql'})
 
-    run("%s syncdb --noinput --settings=%s" %
-        (django_admin_cmd, settings_module),
-        cwd=working_dir)
-
     if django_south:
         south_config_file = os.path.join(settings_dir_path, '50-south.py')
         process_template('south.tmpl', {}, south_config_file)
 
-        run("%s migrate --settings=%s" %
-            (django_admin_cmd, settings_module),
-            cwd=working_dir)
+    update_database_schema()
 
     run('chown -R %s:%s %s' % (wsgi_user, wsgi_group, vcs_clone_dir))
 
@@ -633,7 +634,6 @@
 @hooks.hook('amqp-relation-changed')
 def amqp_relation_changed():
     os.environ['DJANGO_SETTINGS_MODULE'] = settings_module
-    django_admin_cmd = find_django_admin_cmd()
 
     host = relation_get("hostname")
     if not host:
@@ -653,13 +653,7 @@
 
     run('chown -R %s:%s %s' % (wsgi_user, wsgi_group, vcs_clone_dir))
 
-    run("%s syncdb --noinput --settings=%s" %
-        (django_admin_cmd, settings_module),
-        cwd=working_dir)
-
-    if django_south:
-        run("%s migrate --settings=%s" % (django_admin_cmd, settings_module),
-            cwd=working_dir)
+    update_database_schema()
 
     # Trigger WSGI reloading
     for relid in relation_ids('wsgi'):

=== modified file 'templates/conf_injection.tmpl'
--- templates/conf_injection.tmpl	2014-04-16 16:06:59 +0000
+++ templates/conf_injection.tmpl	2015-01-08 00:39:19 +0000
@@ -10,4 +10,5 @@
     from django.conf.global_settings import *
 
 for f in conffiles:
-    exec(open(abspath(f)).read())
+    filename = abspath(f)
+    exec(compile(open(filename, "rb").read(), filename, 'exec'))

=== modified file 'templates/urls_injection.tmpl'
--- templates/urls_injection.tmpl	2014-04-16 16:06:59 +0000
+++ templates/urls_injection.tmpl	2015-01-08 00:39:19 +0000
@@ -10,4 +10,5 @@
     urlpatterns = ()
 
 for f in conffiles:
-    exec(open(abspath(f)).read())
+    filename = abspath(f)
+    exec(compile(open(filename, "rb").read(), filename, 'exec'))