launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/settings-cleanup into lp:maas with lp:~rvb/maas/cleanup-dev-settings as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/settings-cleanup/+merge/95405

This branch:

- adds the apache config file used in production to serve the MaaS server pages from apache: contrib/maas-http.conf
  This required supporting Django's FORCE_SCRIPT_NAME setting
- fixes the settings in such a way that the app admin/admindoc are only installed if DEBUG=True

Drive-by fixes:
- src/maasserver/provisioning.py had to be changed to avoid a circular import
- src/maas/urls.py had to by fixed:
   - admin/doc/ should be put before admin
   - staticfiles_urlpatterns should be appended to the active urls only if STATIC_LOCAL_SERVE is True
-- 
https://code.launchpad.net/~rvb/maas/settings-cleanup/+merge/95405
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/settings-cleanup into lp:maas.

=== added directory 'contrib'
=== added file 'contrib/maas-http.conf'
--- contrib/maas-http.conf	1970-01-01 00:00:00 +0000
+++ contrib/maas-http.conf	2012-03-01 16:33:23 +0000
@@ -0,0 +1,18 @@
+WSGIScriptAlias /MaaS  /usr/share/pyshared/maas/wsgi.py
+
+# Serve files from staticfiles.
+Alias /MaaS/static/ /usr/share/maas/web/static/
+<Directory /usr/share/maas/web/static/>
+    SetHandler None
+    ExpiresActive On
+    ExpiresDefault "access plus 365 days"
+</Directory>
+
+# This can be safely removed once Django 1.4 is used: admin media
+# will be served using staticfiles.
+Alias /MaaS/static/admin/ /usr/share/pyshared/django/contrib/admin/media/
+<Directory /usr/share/pyshared/django/contrib/admin/media/>
+    SetHandler None
+    ExpiresActive On
+    ExpiresDefault "access plus 365 days"
+</Directory>

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-03-01 16:33:23 +0000
+++ src/maas/settings.py	2012-03-01 16:33:23 +0000
@@ -19,6 +19,17 @@
 
 django.template.add_to_builtins('django.templatetags.future')
 
+DEBUG = False
+
+# Used to set a prefix in front of every URL.
+FORCE_SCRIPT_NAME = None
+
+# Allow the user to override DEBUG and FORCE_SCRIPT_NAME in local_settings.
+try:
+    from local_settings import DEBUG, FORCE_SCRIPT_NAME
+except:
+    pass
+
 ADMINS = (
     # ('Your Name', 'your_email@xxxxxxxxxxx'),
 )
@@ -30,6 +41,15 @@
 
 LOGOUT_URL = '/'
 LOGIN_REDIRECT_URL = '/'
+LOGIN_URL = '/accounts/login/'
+
+if FORCE_SCRIPT_NAME is not None:
+    LOGOUT_URL = FORCE_SCRIPT_NAME + LOGOUT_URL
+    LOGIN_REDIRECT_URL = FORCE_SCRIPT_NAME + LOGIN_REDIRECT_URL
+    LOGIN_URL = FORCE_SCRIPT_NAME + LOGIN_URL
+    # ADMIN_MEDIA_PREFIX will be deprecated in Django 1.4.
+    # Admin's media will be served using staticfiles instead.
+    ADMIN_MEDIA_PREFIX = FORCE_SCRIPT_NAME
 
 API_URL_REGEXP = '^/api/1[.]0/'
 METADATA_URL_REGEXP = '^/metadata/'
@@ -39,14 +59,6 @@
 # maasserver.middleware.APIErrorsMiddleware)
 PISTON_DISPLAY_ERRORS = False
 
-DEBUG = False
-
-# Allow the user to override DEBUG in local_settings.
-try:
-    from local_settings import DEBUG
-except:
-    pass
-
 TEMPLATE_DEBUG = DEBUG
 YUI_DEBUG = DEBUG
 YUI_VERSION = '3.4.1'
@@ -113,11 +125,15 @@
 # URL prefix for static files.
 # Example: "http://media.lawrence.com/static/";
 STATIC_URL = '/static/'
+if FORCE_SCRIPT_NAME is not None:
+    STATIC_URL = FORCE_SCRIPT_NAME + STATIC_URL
 
 # URL prefix for admin static files -- CSS, JavaScript and images.
 # Make sure to use a trailing slash.
 # Examples: "http://foo.com/static/admin/";, "/static/admin/".
 ADMIN_MEDIA_PREFIX = '/static/admin/'
+if FORCE_SCRIPT_NAME is not None:
+    ADMIN_MEDIA_PREFIX = FORCE_SCRIPT_NAME + ADMIN_MEDIA_PREFIX
 
 # Additional locations of static files
 STATICFILES_DIRS = (
@@ -185,7 +201,6 @@
     'django.contrib.sites',
     'django.contrib.messages',
     'django.contrib.staticfiles',
-    'django.contrib.admin',
     'maasserver',
     'metadataserver',
     'piston',
@@ -193,6 +208,11 @@
     # 'django.contrib.admindocs',
 )
 
+if DEBUG:
+    INSTALLED_APPS += (
+        'django.contrib.admin',
+        'django.contrib.admindocs',
+    )
 # A sample logging configuration. The only tangible logging
 # performed by this configuration is to send an email to
 # the site admins on every HTTP 500 error.

=== modified file 'src/maas/urls.py'
--- src/maas/urls.py	2012-02-07 18:14:14 +0000
+++ src/maas/urls.py	2012-03-01 16:33:23 +0000
@@ -31,13 +31,13 @@
             {'document_root': settings.MEDIA_ROOT}),
     )
 
-urlpatterns += staticfiles_urlpatterns()
+    urlpatterns += staticfiles_urlpatterns()
 
 if settings.DEBUG:
     from django.contrib import admin
     admin.autodiscover()
 
     urlpatterns += patterns('',
+        (r'^admin/doc/', include('django.contrib.admindocs.urls')),
         (r'^admin/', include(admin.site.urls)),
-        (r'^admin/doc/', include('django.contrib.admindocs.urls')),
     )

=== modified file 'src/maasserver/middleware.py'
--- src/maasserver/middleware.py	2012-02-24 17:37:01 +0000
+++ src/maasserver/middleware.py	2012-03-01 16:33:23 +0000
@@ -34,6 +34,19 @@
 from maasserver.exceptions import MaaSAPIException
 
 
+def get_relative_path(path):
+    """If the url prefix settings.FORCE_SCRIPT_NAME is not None: strip the
+    prefix from the given path.
+    """
+    prefix = settings.FORCE_SCRIPT_NAME
+    if prefix is None:
+        return path
+    elif path.startswith(prefix):
+        return path[len(prefix):]
+    else:
+        raise Exception("Prefix '%s' not in path '%s'" % (prefix, path))
+
+
 class AccessMiddleware:
     """Protect access to views.
 
@@ -64,12 +77,12 @@
 
     def process_request(self, request):
         # Public urls.
-        if self.public_urls.match(request.path):
+        if self.public_urls.match(get_relative_path(request.path)):
             return None
         else:
             if request.user.is_anonymous():
                 return HttpResponseRedirect("%s?next=%s" % (
-                    self.login_url, urlquote_plus(request.path)))
+                    settings.LOGIN_URL, urlquote_plus(request.path)))
             else:
                 return None
 
@@ -101,7 +114,7 @@
 
     def process_exception(self, request, exception):
         """Django middleware callback."""
-        if not self.path_matcher.match(request.path):
+        if not self.path_matcher.match(get_relative_path(request.path)):
             # Not a path we're handling exceptions for.
             return None
 

=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	2012-02-28 16:52:02 +0000
+++ src/maasserver/provisioning.py	2012-03-01 16:33:23 +0000
@@ -28,7 +28,6 @@
     MACAddress,
     Node,
     )
-from metadataserver.models import NodeKey
 
 
 def get_provisioning_api_proxy():
@@ -68,6 +67,8 @@
     :return: A dict containing metadata information that will be seeded to
         the node, so that it can access the metadata service.
     """
+    # Circular import.
+    from metadataserver.models import NodeKey
     token = NodeKey.objects.get_token_for_node(node)
     credentials = urlencode({
         'oauth_consumer_key': token.consumer.key,