Merge lp:~sinzui/charmworld/support-gui-charm-urls into lp:~juju-jitsu/charmworld/trunk
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | 384 |
Merged at revision: | 373 |
Proposed branch: | lp:~sinzui/charmworld/support-gui-charm-urls |
Merge into: | lp:~juju-jitsu/charmworld/trunk |
Diff against target: |
353 lines (+206/-12) 5 files modified
charmworld/routes.py (+13/-1) charmworld/views/bundles.py (+11/-0) charmworld/views/charms.py (+37/-10) charmworld/views/tests/test_bundles.py (+21/-1) charmworld/views/tests/test_charms.py (+124/-0) |
To merge this branch: | bzr merge lp:~sinzui/charmworld/support-gui-charm-urls |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Benji York (community) | Approve | ||
Review via email: mp+182924@code.launchpad.net |
Commit message
Support juju-gui URLs in charmworld.
Description of the change
Support juju-gui URLs in charmworld.
RULES
Pre-
* Juju-gui wants to delegate requests for charm data when the client
is not supported -- juju-gui supports a small set of JS-enabled
browsers.
* Users need to see charm (and bundle) details to learn what they
do and how to use them. We expect users and sometimes bots to
find these links and need access to discover juju or develop
with it.
* The gui or it's front-end server will forward the request to
charmworld. Charmworld needs to understand the URL.
* Charmworld supports unversioned user urls now. It must support:
* -{rev} in any charm url.
* /{series}/
* ^ These urls do not conflict with current urls so moving the old urls
can be postponed.
QA
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
* Visit http://
IMPLEMENTATION
I added routing tests for charms before I changed code. I discovered that
the charm json views were missing a content type. While content_type was
passed to the Response, it was clobbered by the headerlist. The fix was
to include the content-type in the header list.
I extracted a helper from a pattern I saw in several charm views.
_reconcile_
mistaken for a revision. I added a rule to handle the HEAD case that
juju gui's apache adds.
The main difference between charmworld URLs and juju-gui URLs is the
revision and the lack of namespacing. Adding revision to personal branches
and bundles was trivial. Reviewed charms was tricky. Making the charms
the last rules ensures the current urls are not misconstrued to be charm
series. Since bundles is both a namespace and a series, there was no conflict.
I did add a revision to the official bundle json view to avoid conflicts with
the route I believe the GUI will use; this make the official json view
consistent with the person json view.
The branch looks good. The tests especially nice.
On lines 233, 255, and 262 you could use self.assertIn and get slightly
better diagnostics if the assertion ever fails.
Unpacking the comments in the routing tests would be nice. I can pretty personal_ charm_with_ revision' s comment of
much understand them in the context of the branch, but I suspect someone
coming in cold would have to ruminate a bit to see what they are saying.
For example, test_route_
# /~owner/ series/ charm-1 route find the charm.
Might be expanded to
# A non-promulgated charm's route of the form /~owner/ series/ charm-1 series/ charm.
# specifies the charm with a charm store ID of
# cs:~owner/