Update bundle user url to be /bundle/$id-bits.

Update bundle user url to be /bundle/$id-bits.

- Update the api to provide that url as part of the bundle metadata so the gui
can link to it as a valid quickstart end point.
- Drive by to skip the test that fails with the known bug issue. 
- Rename the TestApi2 and 3 back to not have the Z since we're just skipping
vs working around the issue.

https://code.launchpad.net/~rharding/charmworld/move-bundle-url/+merge/193851

(do not edit description out of merge proposal)

[rharding, 2013-11-04 21:28:53]

Please take a look.

[rharding, 2013-11-04 21:43:46]

Please take a look.

[rharding, 2013-11-04 21:46:33]

Reviewer comments added.

https://codereview.appspot.com/21370044/diff/20001/charmworld/routes.py
File charmworld/routes.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/routes.py#newco...
charmworld/routes.py:57: "/bundle/~{owner}/{basket}/{rev}/{bundle}/json")
this could probably go into some sort of /bundle/*bits but leaving as simple
case of moving the url for the moment.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/api/__ini...
File charmworld/views/api/__init__.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/api/__ini...
charmworld/views/api/__init__.py:159: def _bundle_result(cls, bundle, db,
route_url):
route_url is needed to build the url to the person bundle json route we're
providing as an endpoint for the deployer/quickstart to use.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
File charmworld/views/tests/test_api.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:768: self.enable_routes()
because of the need to generate the route for the deployer_file_url we need to
enable routes on more tests.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:841:
u'http://example.com/bundle/%7Ebac/bat/4/byobu/json',
I'm so sorry, but the linter...he hates me.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:1585: class TestAPI3(TestAPI, API3Mixin):
remove the stupid Z. Just skipping the bad test for now. See later files.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
File charmworld/views/tests/test_misc.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_misc.py:54: self.skipTest("See bug #1237025")
Skipping this as the new enable_routes() caused it to fail over here for no good
reason. Will look at as a follow up after bundles are released.

[benji, 2013-11-05 12:55:45]

LGTM

https://codereview.appspot.com/21370044/diff/20001/charmworld/routes.py
File charmworld/routes.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/routes.py#newco...
charmworld/routes.py:57: "/bundle/~{owner}/{basket}/{rev}/{bundle}/json")
On 2013/11/04 21:46:34, rharding wrote:
> this could probably go into some sort of /bundle/*bits but leaving as simple
> case of moving the url for the moment.

If I'm understanding you correctly, I think I like it better this way.  It is
easier to see the pattern matching spelled out in routes rather than in code.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/api/__ini...
File charmworld/views/api/__init__.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/api/__ini...
charmworld/views/api/__init__.py:429: 'personal-bundle-json-revision',
**route_info)
Since route_info isn't used subsequently, I don't see why it is created instead
of just passing its contents as keyword arguments.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
File charmworld/views/tests/test_api.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:1: # Copyright<enter change description here
if sensible> 2012, 2013 Canonical
Something bad happened here.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:841:
u'http://example.com/bundle/%7Ebac/bat/4/byobu/json',
On 2013/11/04 21:46:34, rharding wrote:
> I'm so sorry, but the linter...he hates me.

Eww.  That is bad.

It wouldn't accept the value being indented four more spaces?

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:1585: class TestAPI3(TestAPI, API3Mixin):
On 2013/11/04 21:46:34, rharding wrote:
> remove the stupid Z. Just skipping the bad test for now. See later files.

I like the explicitness of the new approach better.

[rharding, 2013-11-05 13:12:54]

Thanks for the review. I made a couple of small updates.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
File charmworld/views/tests/test_api.py (right):

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:841:
u'http://example.com/bundle/%7Ebac/bat/4/byobu/json',
On 2013/11/05 12:55:46, benji wrote:
> On 2013/11/04 21:46:34, rharding wrote:
> > I'm so sorry, but the linter...he hates me.
> 
> Eww.  That is bad.
> 
> It wouldn't accept the value being indented four more spaces?

No, it yelled about bad indentation. I was trapped between the line length and
the indentation fussing. I moved the string out into a variable. I thought about
it yesterday but it seemed ugly to me. Now looking at it, it's much more sane.

https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/tes...
charmworld/views/tests/test_api.py:1585: class TestAPI3(TestAPI, API3Mixin):
On 2013/11/05 12:55:46, benji wrote:
> On 2013/11/04 21:46:34, rharding wrote:
> > remove the stupid Z. Just skipping the bad test for now. See later files.
> 
> I like the explicitness of the new approach better.

Thanks, removed the comments above as well. Missed that :/