Gary Poster has proposed merging lp:~gary/launchpad/bug607961 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #607961 in Launchpad itself: "wadl generation timeout?"
https://bugs.launchpad.net/launchpad/+bug/607961
For more details, see:
https://code.launchpad.net/~gary/launchpad/bug607961/+merge/67246
pre-imp with lifeless, via the bug report; the Apache config example was vetted
by mthaddon.
This branch attempts to fix bug 607961 by allowing us to push the serving of
the wadl files from the app server to Apache.
Deployment of the change should have two phases, and both need to be qa'd.
First, the branch should continue to work as it did before: without Apache
changes, the app server continues to serve the wadl as it did before. This
should also be true for devel machines for reviewers. Simply connecting to
qastaging using launchpadlib should be sufficient, because it immediately gets
the WADL; for safety, we might as well try to do a representative operation
with launchpadlib, but that should be sufficient.
That should be able to make its way to production without blocking anything.
The second stage of tests should be to apply pertinent changes to qastaging's
Apache. http://pastebin.ubuntu.com/639701/ is a developer-box version of the
changes needed, and can be used by a reviewer if desired. The most pertinent
parts of this are lines 3-5 and lines 25-33. Importantly, we also need to
include "FileETag MTime Size" within the Directory section. LOSAs will need to
decide what the DirectoryRoot should point to; however the LOSAs want to
arrange it, it needs to be the lib/canonical/launchpad/apidoc directory of the
deployed branch. Robert also points out that the approach that LOSAs have for
this should "be robust on rollouts (as icing is)"; I think that probably will
happen naturally, but we should clarify that with LOSAs on the RT for this
stage.
All that said, what's going on in this branch?
Serving a static file from Apache is not a big deal, but there are some
wrinkles that needed to be handled. Most of these are outlined by Leonard in
the bug: https://bugs.launchpad.net/launchpad/+bug/607961/comments/6 .
(1) We need to be able to serve different versions of the root resource of a
version depending on the mimetype: json or wadl. Serving html there would be
nice too.
(2) We need to support the "real" wadl mime type and a misspelling that older
versions of launchpadlib used.
(3) We want to continue to support compression and ETags.
(4) Tests need to continue to use a Zope-served version for parallel test runs.
I used Apache's MultiViews for the content negotation (1). That is turned on
in the Apache snippet I give in the pastebin.
http://httpd.apache.org/docs/current/content-negotiation.html .
The mime types need to be known in Apache for this to work. I define wadl and
json in the pastebin. I also define the misspelling (2). I don't care for
this solution, but it was the third I tried. I document the other two in a new
comment in create-lp-wadl-and-apidoc.py, as seen in the diff below.
Compression and ETags (3) are entirely in Apache now. The Etags are set in
Apache to correspond to file size and mtime; and we set the mtime of the
generated wadl and json files to the time of the last commit in the branch, so
they should be stable across multiple Launchpad servers.
For keeping Zope-served versions available for tests (4), I simply had to keep
the previous mechanism working, which I did.
For lint, I made lint happy with my additions, and also acquiesced to most of
its other demands, though I left a few pre-existing issues alone.
./Makefile
32: Line exceeds 78 characters.
289: Line exceeds 78 characters.
448: Line exceeds 78 characters.
./utilities/create-lp-wadl-and-apidoc.py
13: '_pythonpath' imported but unused
To check everything out locally, you can apply the Apache changes if you want
to, and then use curl to verify that you get different resources appropriately,
all with the same Last Modified time. The ETag should be stable, even if you
make clean and make (or just remake the apidoc).
curl -vk https://api.launchpad.dev/beta/ -H "Accept:
application/vnd.sun.wadl+xml"
curl -vk https://api.launchpad.dev/beta/ -H "Accept:
application/vd.sun.wadl+xml"
curl -vk https://api.launchpad.dev/beta/ -H "Accept: application/json"
curl -vk https://api.launchpad.dev/1.0/ -H "Accept:
application/vnd.sun.wadl+xml"
curl -vk https://api.launchpad.dev/1.0/ -H "Accept: application/vd.sun.wadl+xml"
curl -vk https://api.launchpad.dev/1.0/ -H "Accept: application/json"
curl -vk https://api.launchpad.dev/devel/ -H "Accept:
application/vnd.sun.wadl+xml"
curl -vk https://api.launchpad.dev/devel/ -H "Accept:
application/vd.sun.wadl+xml"
curl -vk https://api.launchpad.dev/devel/ -H "Accept: application/json"
These urls should also all work in the browswer: https://api.launchpad.dev/,
https://api.launchpad.dev/beta, https://api.launchpad.dev/beta.html,
https://api.launchpad.dev/devel, https://api.launchpad.dev/devel.html,
https://api.launchpad.dev/1.0, https://api.launchpad.dev/1.0.html .
You should also be able to start up Launchpad and still use launchpadlib to
access the dev instance (``lp-shell dev devel`` for instance).
For QA, again, there should be two stages to this: before Apache has changed
and after. In both cases, all of the above should work smoothly (change
api.launchpad.dev to api.launchpad.net, and change ``lp-shell dev`` to
``lp-shell production``).
That's everything I can think of. Thanks!
--
https://code.launchpad.net/~gary/launchpad/bug607961/+merge/67246
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~gary/launchpad/bug607961 into lp:launchpad.
=== modified file 'Makefile'
--- Makefile 2011-07-02 15:40:24 +0000
+++ Makefile 2011-07-07 20:11:29 +0000
@@ -40,7 +40,7 @@
BZR_VERSION_INFO = bzr-version-info.py
APIDOC_DIR = lib/canonical/launchpad/apidoc
-WADL_TEMPLATE = $(APIDOC_DIR).tmp/wadl-$(LPCONFIG)-%(version)s.xml
+APIDOC_TMPDIR = $(APIDOC_DIR).tmp/
API_INDEX = $(APIDOC_DIR)/index.html
# Do not add bin/buildout to this list.
@@ -78,8 +78,8 @@
rm -rf $(APIDOC_DIR) $(APIDOC_DIR).tmp
mkdir -p $(APIDOC_DIR).tmp
LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py \
- --force "$(WADL_TEMPLATE)"
- mv $(APIDOC_DIR).tmp $(APIDOC_DIR)
+ --force "$(APIDOC_TMPDIR)"
+ mv $(APIDOC_TMPDIR) $(APIDOC_DIR)
apidoc: compile $(API_INDEX)
@@ -486,4 +486,4 @@
reload-apache hosted_branches check_mailman check_config \
jsbuild jsbuild_minify clean_js clean_buildout buildonce_eggs \
build_eggs sprite_css sprite_image css_combine compile \
- check_schema pydoctor clean_logs
+ check_schema pydoctor clean_logs
=== modified file 'lib/canonical/launchpad/ftests/test_wadl_generation.py'
--- lib/canonical/launchpad/ftests/test_wadl_generation.py 2010-10-20 03:14:53 +0000
+++ lib/canonical/launchpad/ftests/test_wadl_generation.py 2011-07-07 20:11:29 +0000
@@ -5,21 +5,17 @@
__metaclass__ = type
-import pkg_resources
-import shutil
-import subprocess
-import tempfile
-
from testtools.matchers import StartsWith
from zope.component import getUtility
-from canonical.launchpad.rest.wadl import generate_wadl, generate_html
-from canonical.launchpad.systemhomes import WebServiceApplication
+from canonical.launchpad.rest.wadl import (
+ generate_json,
+ generate_wadl,
+ )
from canonical.testing import LaunchpadFunctionalLayer
from lazr.restful.interfaces import IWebServiceConfiguration
from lp.testing import TestCase
-from lp.testing.matchers import Contains
class SmokeTestWadlAndDocGeneration(TestCase):
@@ -32,3 +28,9 @@
for version in config.active_versions:
wadl = generate_wadl(version)
self.assertThat(wadl[:40], StartsWith('<?xml '))
+
+ def test_json(self):
+ config = getUtility(IWebServiceConfiguration)
+ for version in config.active_versions:
+ json = generate_json(version)
+ self.assertThat(json, StartsWith('{"'))
=== modified file 'lib/canonical/launchpad/rest/wadl.py'
--- lib/canonical/launchpad/rest/wadl.py 2011-01-13 17:16:28 +0000
+++ lib/canonical/launchpad/rest/wadl.py 2011-07-07 20:11:29 +0000
@@ -20,15 +20,16 @@
from canonical.launchpad.webapp.vhosts import allvhosts
-def generate_wadl(version):
- """Generate the WADL for the given version of the web service."""
+def _generate_web_service_root(version, mimetype):
+ """Generate the webservice description for the given version and mimetype.
+ """
url = urlparse.urljoin(allvhosts.configs['api'].rooturl, version)
# Since we want HTTPS URLs we have to munge the request URL.
url = url.replace('http://', 'https://')
request = WebServiceTestRequest(version=version, environ={
'SERVER_URL': url,
'HTTP_HOST': allvhosts.configs['api'].hostname,
- 'HTTP_ACCEPT': 'application/vd.sun.wadl+xml',
+ 'HTTP_ACCEPT': mimetype,
})
# We then bypass the usual publisher processing by associating
# the request with the WebServicePublication (usually done by the
@@ -39,6 +40,16 @@
return request.publication.getApplication(request)(request)
+def generate_wadl(version):
+ """Generate the WADL for the given version of the web service."""
+ return _generate_web_service_root(version, 'application/vd.sun.wadl+xml')
+
+
+def generate_json(version):
+ """Generate the JSON for the given version of the web service."""
+ return _generate_web_service_root(version, 'application/json')
+
+
def generate_html(wadl_filename, suppress_stderr=True):
"""Given a WADL file generate HTML documentation from it."""
# If we're supposed to prevent the subprocess from generating output on
@@ -59,4 +70,3 @@
raise subprocess.CalledProcessError(process.returncode, args)
return output
-
=== modified file 'lib/canonical/launchpad/systemhomes.py'
--- lib/canonical/launchpad/systemhomes.py 2011-01-21 21:42:40 +0000
+++ lib/canonical/launchpad/systemhomes.py 2011-07-07 20:11:29 +0000
@@ -381,12 +381,15 @@
cached_wadl = {}
+ # This should only be used by devel instances: production serves root
+ # WADL (and JSON) from the filesystem.
+
@classmethod
def cachedWADLPath(cls, instance_name, version):
"""Helper method to calculate the path to a cached WADL file."""
return os.path.join(
os.path.dirname(os.path.normpath(__file__)),
- 'apidoc', 'wadl-%s-%s.xml' % (instance_name, version))
+ 'apidoc', version, '%s.wadl' % (instance_name,))
def toWADL(self):
"""See `IWebServiceApplication`.
=== modified file 'utilities/create-lp-wadl-and-apidoc.py'
--- utilities/create-lp-wadl-and-apidoc.py 2010-11-19 14:22:15 +0000
+++ utilities/create-lp-wadl-and-apidoc.py 2011-07-07 20:11:29 +0000
@@ -10,59 +10,120 @@
% LPCONFIG=development bin/py utilities/create-lp-wadl-and-apidoc.py \\
"lib/canonical/launchpad/apidoc/wadl-development-%(version)s.xml"
"""
-import _pythonpath # Not lint, actually needed.
+import _pythonpath # Not lint, actually needed.
from multiprocessing import Process
import optparse
import os
import sys
+import bzrlib
+from bzrlib.branch import Branch
from zope.component import getUtility
from zope.pagetemplate.pagetemplatefile import PageTemplateFile
-from canonical.launchpad.rest.wadl import generate_wadl, generate_html
+from canonical.launchpad.rest.wadl import (
+ generate_html,
+ generate_json,
+ generate_wadl,
+ )
from canonical.launchpad.scripts import execute_zcml_for_scripts
from canonical.launchpad.systemhomes import WebServiceApplication
from lazr.restful.interfaces import IWebServiceConfiguration
-def write(filename, content):
+def write(filename, content, timestamp):
"""Replace the named file with the given string."""
f = open(filename, 'w')
f.write(content)
f.close()
-
-
-def make_files(path_template, directory, version, force):
- wadl_filename = path_template % {'version': version}
- # If the WADL file doesn't exist or we're being forced to regenerate
- # it...
- if (not os.path.exists(wadl_filename) or force):
- print "Writing WADL for version %s to %s." % (
- version, wadl_filename)
- write(wadl_filename, generate_wadl(version))
- else:
- print "Skipping already present WADL file:", wadl_filename
+ os.utime(filename, (timestamp, timestamp)) # (atime, mtime)
+
+
+def make_files(directory, version, timestamp, force):
+ version_directory = os.path.join(directory, version)
+ base_filename = os.path.join(version_directory, os.environ['LPCONFIG'])
+ wadl_filename = base_filename + '.wadl'
+ json_filename = base_filename + '.json'
+ html_filename = os.path.join(directory, version + ".html")
+ wadl_index = os.path.join(version_directory, 'index.wadl')
+ json_index = os.path.join(version_directory, 'index.json')
+ html_index = os.path.join(version_directory, 'index.html')
+ brokenwadl_index = os.path.join(version_directory, 'index.brokenwadl')
+
+ # Make sure we have our dir.
+ if not os.path.exists(version_directory):
+ # We expect the main directory to exist.
+ os.mkdir(version_directory)
+
+ # Make wadl and json files.
+ for src, dest, gen, name in (
+ (wadl_filename, wadl_index, generate_wadl, 'WADL'),
+ (json_filename, json_index, generate_json, 'JSON')):
+ # If the src doesn't exist or we are forced to regenerate it...
+ if (not os.path.exists(src) or force):
+ print "Writing %s for version %s to %s." % (
+ name, version, src)
+ write(src, gen(version), timestamp)
+ else:
+ print "Skipping already present %s file: %s" % (
+ name, src)
+ # Make "index" symlinks, removing any preexisting ones.
+ if os.path.exists(dest):
+ os.remove(dest)
+ os.symlink(os.path.basename(src), dest)
+
+ # Make the brokenwadl symlink. This is because we need to support a
+ # misspelled wadl mimetype that some legacy launchpadlib versions used.
+ # Multiple attempts have been made to make this unnecessary, and removing
+ # it is welcome. In particular, these two approaches were attempted in
+ # Apache.
+ #
+ # Approach 1 (a variant of example 4
+ # from http://httpd.apache.org/docs/2.0/mod/mod_headers.html)
+ # SetEnvIf Accept \Qapplication/vd.sun.wadl+xml\E X_WADL_MIME
+ # RequestHeader set Accept "application/vnd.sun.wadl+xml" env=X_WADL_MIME
+ # This, at least in combination with Apache's MultiViews, doesn't work
+ # in developer tests.
+ #
+ # Approach 2:
+ # In mime.conf,
+ # AddType application/vnd.sun.wadl+xml .wadl
+ # AddType application/vd.sun.wadl+xml .wadl
+ # In developer tests, it seems Apache only allows a single mime type
+ # for a given extension.
+ #
+ # Therefore, the approach we use is
+ # AddType application/vnd.sun.wadl+xml .wadl
+ # AddType application/vd.sun.wadl+xml .brokenwadl
+ # We support that here.
+ if not os.path.exists(brokenwadl_index):
+ os.symlink(os.path.basename(wadl_index), brokenwadl_index)
# Now, convert the WADL into an human-readable description and
# put the HTML in the same directory as the WADL.
- html_filename = os.path.join(directory, version + ".html")
# If the HTML file doesn't exist or we're being forced to regenerate
# it...
if (not os.path.exists(html_filename) or force):
print "Writing apidoc for version %s to %s" % (
version, html_filename)
write(html_filename, generate_html(wadl_filename,
- suppress_stderr=False))
+ suppress_stderr=False), timestamp)
else:
print "Skipping already present HTML file:", html_filename
-
-def main(path_template, force=False):
- WebServiceApplication.cached_wadl = None # do not use cached file version
+ # Symlink the top-level version html in the version directory for
+ # completeness.
+ if not os.path.exists(html_index):
+ os.symlink(
+ os.path.join(os.path.pardir, os.path.basename(html_filename)),
+ html_index)
+
+
+def main(directory, force=False):
+ WebServiceApplication.cached_wadl = None # do not use cached file version
execute_zcml_for_scripts()
config = getUtility(IWebServiceConfiguration)
- directory = os.path.dirname(path_template)
# First, create an index.html with links to all the HTML
# documentation files we're about to generate.
@@ -73,11 +134,19 @@
f = open(index_filename, 'w')
f.write(template(config=config))
+ # Get the time of the last commit. We will use this as the mtime for the
+ # generated files so that we can safely use it as part of Apache's etag
+ # generation in the face of multiple servers/filesystems.
+ with bzrlib.initialize():
+ branch = Branch.open(os.path.dirname(os.path.dirname(__file__)))
+ timestamp = branch.repository.get_revision(
+ branch.last_revision()).timestamp
+
# Start a process to build each set of WADL and HTML files.
processes = []
for version in config.active_versions:
p = Process(target=make_files,
- args=(path_template, directory, version, force))
+ args=(directory, version, timestamp, force))
p.start()
processes.append(p)
@@ -89,7 +158,7 @@
def parse_args(args):
- usage = "usage: %prog [options] PATH_TEMPLATE"
+ usage = "usage: %prog [options] DIR"
parser = optparse.OptionParser(usage=usage)
parser.add_option(
"--force", action="store_true",
@@ -97,7 +166,7 @@
parser.set_defaults(force=False)
options, args = parser.parse_args(args)
if len(args) != 2:
- parser.error("A path template is required.")
+ parser.error("A directory is required.")
return options, args
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
