Merge lp:~stevenk/convoy/exportable-app into lp:convoy

[Steve Kowalik]

This branch moves the inner WSGI application handler from inside combo_app() to be a fully fledged method that can be called outside of combo_app.

The reason for this is we'd like to override the root directory using SetEnv in the Apache config, which gets passed into the WSGI call in the environ dict. Since the application handler used by combo_app() isn't exported, I can't use combo_app, since the start_response variable can't be passed around.

I've tested use of combo_app() and _application() before submitting this MP, and both look to work great.

[Sidnei da Silva (sidnei)]

I'm really confused as to why this is needed. In any case, if the intention is to call _application externally, making this some sort of public API it should definitely not start with an underscore, and a test for it should be added.

review: Needs Fixing

[lp:~stevenk/convoy/exportable-app]

21. By Steve Kowalik on 2012-02-07

Rename _application to application, write a test, and fix up setup.py not to
install tests.

[Steve Kowalik (stevenk)]

I've added a test, and I've chopped off the underscore.

Hopefully, the WSGI file I'm going to use with this will help explain:

from convoy.combo import application

def application(environ, start_response):
    root = environ.get('CONVOY_ROOT', '/srv/launchpad.dev/convoy')
    return application(environ, start_response, root=root)

As you can see, we want to get the root from the environment passed to the WSGI app. I can't use combo_app() for this, since WSGI is expecting the same environ and start_response to be used, and before this branch, they were hidden inside combo_app() and couldn't be set by callers.

The above snippet allows me to use the same WSGI for local development, our staging environments and production by use of Apache's SetEnv directive.

[Steve Kowalik (stevenk)]

One thing I meant to add is I'm happy to split out the setup.py changes into a separate smaller branch if you wish.

[Sidnei da Silva (sidnei)]

Surely there's some misunderstanding going on here. Take a look at the following script that emulates your usage, and see how combo_app works just fine as-is, you just need to call it to get an application and then call the application.

https://pastebin.canonical.com/59582/

[Steve Kowalik (stevenk)]

Right, that was the piece I was missing. I was trying for hours yesterday to get this working, and the diff on this MP was the only way I could get it working. Thanks to your pastebin, I don't need this change, and I'll set this MP to rejected.

Unmerged revisions

21. By Steve Kowalik on 2012-02-07

Rename _application to application, write a test, and fix up setup.py not to
install tests.

20. By Steve Kowalik on 2012-02-06

Split out the actual WSGI application function to one that can be exported.
Bump version.

=== modified file 'convoy/combo.py'
--- convoy/combo.py	2012-01-27 03:37:59 +0000
+++ convoy/combo.py	2012-02-07 05:22:20 +0000
@@ -16,6 +16,7 @@
 
 
 import cgi
+from functools import partial
 import os
 import re
 import urlparse
@@ -122,23 +123,27 @@
     C{Content-Type} header.
     """
     root = os.path.abspath(root)
-    def app(environ, start_response, root=root):
-        path_hint = parse_path_hint(environ['PATH_INFO'])
-        fnames = parse_qs(environ["QUERY_STRING"])
-        content_type = "text/plain"
-        if fnames:
-            if fnames[0].endswith(".js"):
-                content_type = "text/javascript"
-            elif fnames[0].endswith(".css"):
-                content_type = "text/css"
-        else:
-            start_response("404 Not Found", [("Content-Type", content_type)])
-            return ("Not Found",)
-        start_response("200 OK", [("Content-Type", content_type)])
-
-        # take any prefix in the url route into consideration for the root to
-        # find files at
-        updated_root = os.path.join(root, path_hint)
-        return combine_files(fnames, updated_root, resource_prefix,
-                             rewrite_urls=rewrite_urls)
-    return app
+    return partial(application, root=root, resource_prefix=resource_prefix,
+        rewrite_urls=rewrite_urls)
+
+
+def application(environ, start_response, root="", resource_prefix="",
+        rewrite_urls=True):
+    path_hint = parse_path_hint(environ['PATH_INFO'])
+    fnames = parse_qs(environ["QUERY_STRING"])
+    content_type = "text/plain"
+    if fnames:
+        if fnames[0].endswith(".js"):
+            content_type = "text/javascript"
+        elif fnames[0].endswith(".css"):
+            content_type = "text/css"
+    else:
+        start_response("404 Not Found", [("Content-Type", content_type)])
+        return ("Not Found",)
+    start_response("200 OK", [("Content-Type", content_type)])
+
+    # take any prefix in the url route into consideration for the root to
+    # find files at
+    updated_root = os.path.join(root, path_hint)
+    return combine_files(fnames, updated_root, resource_prefix,
+                         rewrite_urls=rewrite_urls)

=== modified file 'convoy/tests/test_combo.py'
--- convoy/tests/test_combo.py	2012-01-27 03:37:59 +0000
+++ convoy/tests/test_combo.py	2012-02-07 05:22:20 +0000
@@ -23,10 +23,8 @@
 import mocker
 from paste.fixture import TestApp
 
-from convoy.combo import combo_app
-from convoy.combo import combine_files
-from convoy.combo import parse_path_hint
-from convoy.combo import parse_url
+from convoy.combo import (
+    application, combo_app, combine_files, parse_path_hint, parse_url)
 
 class ComboTestBase(object):
 
@@ -483,5 +481,44 @@
             ["yui/yui-min.js"])), status=200)
         self.assertEquals(res.body.strip(), expected2)
 
+    def test_application_can_inject_root(self):
+        """If root is passed into application, it can be done per-request."""
+        first_root = self.makeDir()
+        second_root = self.makeDir()
+
+        self.makeSampleFile(
+            first_root,
+            os.path.join("yui", "yui-min.js"),
+            "/* yui-min */"),
+
+        self.makeSampleFile(
+            second_root,
+            os.path.join("yui", "yui-min.js"),
+            "/* yui-min-2 */"),
+
+        expected = "\n".join(("/* yui/yui-min.js */",
+                              "/* yui-min */"))
+        expected2 = "\n".join(("/* yui/yui-min.js */",
+                               "/* yui-min-2 */"))
+    
+        def app(environ, start_response):
+            root = environ.get("CONVOY_ROOT")
+            return application(environ, start_response, root=root)
+
+        environ = {'CONVOY_ROOT': first_root}
+        app = TestApp(app)
+
+        res = app.get(
+            "/+combo/?yui/yui-min.js", status=200, extra_environ=environ)
+        self.assertEquals(res.body.strip(), expected)
+
+        environ['CONVOY_ROOT'] = second_root
+        res = app.get(
+            "/+combo/?yui/yui-min.js", status=200, extra_environ=environ)
+        self.assertEquals(res.body.strip(), expected2)
+
+
+
+
 def test_suite():
     return defaultTestLoader.loadTestsFromName(__name__)

=== modified file 'setup.py'
--- setup.py	2012-01-27 03:37:59 +0000
+++ setup.py	2012-02-07 05:22:20 +0000
@@ -22,14 +22,13 @@
 # declare our dependency on epsilon.
 extra_setup_args = {}
 try:
-    import setuptools
     from setuptools import find_packages
     extra_setup_args = {
         'install_requires': ['Paste'],
         'tests_require': ['nose', 'mocker']
         }
 except ImportError:
-    def find_packages():
+    def find_packages(excludes=None):
         """
         Compatibility wrapper.
 
@@ -43,12 +42,12 @@
 
 setup(
     name="convoy",
-    version="0.2.1",
+    version="0.2.2",
     description="A combo WSGI application for use with YUI",
     author="Canonical Javascripters",
     url="https://launchpad.net/convoy",
     license="AGPL",
-    packages=find_packages(),
+    packages=find_packages(exclude=('convoy.tests',)),
     include_package_data=True,
     zip_safe=False,
     test_suite="nose.collector",