Merge lp://staging/~gary/launchpad/bug607961 into lp://staging/launchpad

Proposed by Gary Poster
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 13394
Proposed branch: lp://staging/~gary/launchpad/bug607961
Merge into: lp://staging/launchpad
Diff against target: 320 lines (+125/-41)
5 files modified
Makefile (+4/-4)
lib/canonical/launchpad/ftests/test_wadl_generation.py (+10/-8)
lib/canonical/launchpad/rest/wadl.py (+14/-4)
lib/canonical/launchpad/systemhomes.py (+4/-1)
utilities/create-lp-wadl-and-apidoc.py (+93/-24)
To merge this branch: bzr merge lp://staging/~gary/launchpad/bug607961
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+67246@code.staging.launchpad.net

Commit message

[r=jcsackett][bug=607961] Prepare to serve wadl files (and other static webservice bits) from Apache.

Description of the change

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-34. 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!

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks very nice; I particularly like the timestamp for the etags bit.

review: Approve
Revision history for this message
Robert Collins (lifeless) wrote :

I have one small thought: if its not already document, document that
wadl creation is effectively .PHONY: we always rebuild it because (a)
figuring its deps is crazy hard, and (new) (b) we clamp its timestamp
to the last commit, so make cannot determine 'needs rebuild' anyway.

Revision history for this message
Gary Poster (gary) wrote :

Good point, Robert (though you did make me doublecheck when you said "we always rebuild it"--you have to force a rebuild within a given branch, both before and after my changes; we could maybe make that easier). I'll add a comment to the Makefile in another branch.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.