charmworld

Merge lp:~bac/charmworld/bundle-redirect into lp:charmworld

  1. bundle-redirect
  2. Merge into trunk
Proposed by Brad Crittenden
Status: Merged
Approved by: Brad Crittenden
Approved revision: 524
Merged at revision: 523
Proposed branch: lp:~bac/charmworld/bundle-redirect
Merge into: lp:charmworld
Diff against target: 374 lines (+114/-68)
7 files modified
charmworld/__init__.py (+2/-2)
charmworld/jobs/config.py (+3/-1)
charmworld/testing/factory.py (+1/-4)
charmworld/views/bundles.py (+19/-5)
charmworld/views/helpers.py (+5/-0)
charmworld/views/tests/test_bundles.py (+77/-56)
default.ini (+7/-0)
To merge this branch: bzr merge lp:~bac/charmworld/bundle-redirect
Reviewer Review Type Date Requested Status
Juju Gui Bot continuous-integration Approve
Brad Crittenden (community) code Approve
Fabrice Matrat (community) Approve
Francesco Banconi Approve
Review via email: mp+249420@code.launchpad.net

Commit message

Redirect bundles.

Description of the change

Redirect bundles to jujucharms.com

QA: a bit hard as getting bundles to load that are visible in jujucharms.com is difficult since 1) many bundles name a specific version of the charm and that version may not be ingestable, 2) jujucharms.com does not seem to have a complete set of available bundles. (?)

But if you can find a local bundle that has a corresponding one in jujucharms.com, test that the redirect works.

In all honesty, I'm not going to feel 100% about this branch until we get it on staging and can do very thorough QA.

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Looks good to me with minor suggestions.
Thank you!

review: Approve
Revision history for this message
Fabrice Matrat (fabricematrat) wrote :

I just wonder if redirecting to a search in jujucharms.com won't be safer ??

Revision history for this message
Brad Crittenden (bac) wrote :

QA:
# Personal bundle
http://127.0.0.1:2464/bundle/~a.rosales/storm/instantstorm
# Personal bundle with revision
http://127.0.0.1:2464/bundle/~a.rosales/storm/5/instantstorm
# Official bundle
http://127.0.0.1:2464/bundle/hadoop/7/cluster

Revision history for this message
Fabrice Matrat (fabricematrat) wrote :

Working great, did browse a few bundles and get redirected.
Code LGTM.

review: Approve
Revision history for this message
Juju Gui Bot (juju-gui-bot) wrote :

FAILED: Autolanding.
No commit message was specified in the merge proposal. Hit 'Add commit message' on the merge proposal web page or follow the link below. You can approve the merge proposal yourself to rerun.
https://code.launchpad.net/~bac/charmworld/bundle-redirect/+merge/249420/+edit-commit-message

review: Needs Fixing (continuous-integration)
Revision history for this message
Brad Crittenden (bac) wrote :

Kick CI

review: Approve (code)
Revision history for this message
Juju Gui Bot (juju-gui-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'charmworld/__init__.py'
--- charmworld/__init__.py 2013-11-14 13:38:24 +0000
+++ charmworld/__init__.py 2015-02-18 14:26:31 +0000
@@ -59,12 +59,12 @@
5959
60 if 'http_cache' not in settings:60 if 'http_cache' not in settings:
61 http_cache = app_settings.get('http_cache')61 http_cache = app_settings.get('http_cache')
62 if http_cache is not None:62 if http_cache:
63 http_cache = int(http_cache)63 http_cache = int(http_cache)
64 settings['http_cache'] = http_cache64 settings['http_cache'] = http_cache
65 if 'http_cache_modifier' in settings:65 if 'http_cache_modifier' in settings:
66 mod = settings['http_cache_modifier']66 mod = settings['http_cache_modifier']
67 if settings['http_cache'] is not None:67 if settings['http_cache']:
68 settings['http_cache'] = int(settings['http_cache'] * mod)68 settings['http_cache'] = int(settings['http_cache'] * mod)
69 # We can't let the new setting to get add_view or it blows up69 # We can't let the new setting to get add_view or it blows up
70 # with an unexpected kwarg.70 # with an unexpected kwarg.
7171
=== modified file 'charmworld/jobs/config.py'
--- charmworld/jobs/config.py 2013-10-10 11:27:55 +0000
+++ charmworld/jobs/config.py 2015-02-18 14:26:31 +0000
@@ -7,12 +7,14 @@
7import shutil7import shutil
8import tempfile8import tempfile
99
10from pyramid.settings import asbool
11
10from charmworld.utils import get_ini12from charmworld.utils import get_ini
1113
1214
13settings = get_ini()15settings = get_ini()
1416
15if settings.get('is_testing', 'false').lower() == 'true':17if asbool(settings.get('is_testing')):
16 CHARM_DIR = tempfile.mkdtemp()18 CHARM_DIR = tempfile.mkdtemp()
17 atexit.register(shutil.rmtree, CHARM_DIR, ignore_errors=True)19 atexit.register(shutil.rmtree, CHARM_DIR, ignore_errors=True)
18else:20else:
1921
=== modified file 'charmworld/testing/factory.py'
--- charmworld/testing/factory.py 2014-05-15 15:57:23 +0000
+++ charmworld/testing/factory.py 2015-02-18 14:26:31 +0000
@@ -375,10 +375,7 @@
375375
376376
377def make_bundle(db, *args, **kwargs):377def make_bundle(db, *args, **kwargs):
378 with_basket = False378 with_basket = kwargs.pop('with_basket', False)
379 if 'with_basket' in kwargs:
380 with_basket = True
381 del kwargs['with_basket']
382379
383 bundle_data = get_bundle_data(*args, **kwargs)380 bundle_data = get_bundle_data(*args, **kwargs)
384 bundle = Bundle(bundle_data)381 bundle = Bundle(bundle_data)
385382
=== modified file 'charmworld/views/bundles.py'
--- charmworld/views/bundles.py 2014-03-20 14:00:11 +0000
+++ charmworld/views/bundles.py 2015-02-18 14:26:31 +0000
@@ -5,6 +5,8 @@
55
6import os.path6import os.path
77
8from pyramid.httpexceptions import HTTPMovedPermanently
9
8from charmworld import cached_view_config10from charmworld import cached_view_config
9from charmworld.models import (11from charmworld.models import (
10 Bundle,12 Bundle,
@@ -15,8 +17,8 @@
15from charmworld.views.api import json_response17from charmworld.views.api import json_response
16from charmworld.views.helpers import (18from charmworld.views.helpers import (
17 find_bundle,19 find_bundle,
18 found,
19 format_change,20 format_change,
21 redirect_url,
20)22)
2123
2224
@@ -123,6 +125,14 @@
123 )125 )
124126
125127
128def _bundle_search_redirect(request):
129 """Return the redirect search location."""
130 return '{url}/q/{basket}?type=bundle'.format(
131 url=redirect_url(request),
132 basket=request.matchdict['basket'],
133 )
134
135
126@cached_view_config(136@cached_view_config(
127 route_name="personal-bundle-revision",137 route_name="personal-bundle-revision",
128 renderer="charmworld:templates/bundle.pt")138 renderer="charmworld:templates/bundle.pt")
@@ -130,8 +140,10 @@
130 route_name="personal-bundle",140 route_name="personal-bundle",
131 renderer="charmworld:templates/bundle.pt")141 renderer="charmworld:templates/bundle.pt")
132def personal_bundle(request):142def personal_bundle(request):
133 bundle = find_bundle(request)143 # For simplicity and safety, all bundles will redirect to a search on the
134 return _bundle_view(request, found(bundle))144 # basket name.
145 location = _bundle_search_redirect(request)
146 raise HTTPMovedPermanently(location=location)
135147
136148
137@cached_view_config(149@cached_view_config(
@@ -159,8 +171,10 @@
159 route_name="official-bundle-revision",171 route_name="official-bundle-revision",
160 renderer="charmworld:templates/bundle.pt")172 renderer="charmworld:templates/bundle.pt")
161def official_bundle(request):173def official_bundle(request):
162 bundle = find_bundle(request, promulgated=True)174 # For simplicity and safety, all bundles will redirect to a search on the
163 return _bundle_view(request, found(bundle))175 # basket name.
176 location = _bundle_search_redirect(request)
177 raise HTTPMovedPermanently(location=location)
164178
165179
166@cached_view_config(180@cached_view_config(
167181
=== modified file 'charmworld/views/helpers.py'
--- charmworld/views/helpers.py 2014-02-10 21:30:20 +0000
+++ charmworld/views/helpers.py 2015-02-18 14:26:31 +0000
@@ -161,3 +161,8 @@
161 else:161 else:
162 spec["owner"] = request.matchdict['owner']162 spec["owner"] = request.matchdict['owner']
163 return Bundle.from_query(spec, request.db)163 return Bundle.from_query(spec, request.db)
164
165
166def redirect_url(request):
167 """Get the redirect_jujucharms configuration setting."""
168 return request.registry.settings.get('redirect_jujucharms')
164169
=== modified file 'charmworld/views/tests/test_bundles.py'
--- charmworld/views/tests/test_bundles.py 2014-03-06 16:01:13 +0000
+++ charmworld/views/tests/test_bundles.py 2015-02-18 14:26:31 +0000
@@ -1,10 +1,12 @@
1# Copyright 2013-2014 Canonical Ltd. This software is licensed under the1# Copyright 2013-2015 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4from StringIO import StringIO4from StringIO import StringIO
5import datetime5import datetime
6import unittest6import unittest
77
8from pyramid.httpexceptions import HTTPMovedPermanently
9
8from charmworld.models import (10from charmworld.models import (
9 Bundle,11 Bundle,
10 DatedMetric,12 DatedMetric,
@@ -152,7 +154,7 @@
152 found = find_bundle(request)154 found = find_bundle(request)
153 self.assertEqual(bundle, found)155 self.assertEqual(bundle, found)
154156
155 def test_multiple_bundles_picks_newest(self):157 def _multiple_bundle_setup(self):
156 basket_rev1 = 'riak/1'158 basket_rev1 = 'riak/1'
157 name = 'xyz'159 name = 'xyz'
158 owner = 'matt'160 owner = 'matt'
@@ -161,12 +163,16 @@
161 basket_rev2 = 'riak/10'163 basket_rev2 = 'riak/10'
162 bundle2, bundle2_data = factory.makeBundle(164 bundle2, bundle2_data = factory.makeBundle(
163 self.db, name=name, owner=owner, basket_with_rev=basket_rev2)165 self.db, name=name, owner=owner, basket_with_rev=basket_rev2)
166 return name, owner, bundle2
167
168 def test_multiple_bundles_picks_newest(self):
169 name, owner, expected_bundle = self._multiple_bundle_setup()
164 request = self.getRequest()170 request = self.getRequest()
165 request.matchdict = dict(bundle=name,171 request.matchdict = dict(bundle=name,
166 basket='riak',172 basket='riak',
167 owner=owner)173 owner=owner)
168 found = find_bundle(request)174 found = find_bundle(request)
169 self.assertEqual(bundle2, found)175 self.assertEqual(expected_bundle, found)
170176
171 def test_found_bundle_promulgated(self):177 def test_found_bundle_promulgated(self):
172 bundle, bundle_data = factory.makeBundle(self.db, promulgated=True)178 bundle, bundle_data = factory.makeBundle(self.db, promulgated=True)
@@ -198,13 +204,10 @@
198 basket=basket,204 basket=basket,
199 bundle=bundle.name,205 bundle=bundle.name,
200 )206 )
201 expected = dict(207 with self.assertRaises(HTTPMovedPermanently) as ctx:
202 series='raring',208 personal_bundle(request)
203 relations={},209 expected = '/q/{}?type=bundle'.format(basket)
204 services={},210 self.assertTrue(ctx.exception.location.endswith(expected))
205 )
206 response = personal_bundle(request)
207 self.assertEqual(expected, response['bundle'].data)
208211
209 def test_personal_bundle_json(self):212 def test_personal_bundle_json(self):
210 basket = 'ball'213 basket = 'ball'
@@ -298,37 +301,63 @@
298 # Test that the route works.301 # Test that the route works.
299 path = '/bundle/~{owner}/{basket}/{bundle}'.format(302 path = '/bundle/~{owner}/{basket}/{bundle}'.format(
300 owner=owner, basket=basket, bundle=bundle_name)303 owner=owner, basket=basket, bundle=bundle_name)
301 resp = self.app.get(path, status=200)304 resp = self.app.get(path, status=301)
302 self.assertIn(bundle.owner, resp.body)305 expected = '/q/{}?type=bundle'.format(basket)
303 self.assertIn(bundle.data['series'], resp.body)306 self.assertTrue(resp.location.endswith(expected))
304 self.assertIn(bundle.basket_name, resp.body)307
305 self.assertIn(bundle.owner, resp.body)308 def test_personal_bundle_route_multi_bundle_basket(self):
306 # Ensure the short url to the branch on Launchpad is in the body.309 owner = 'ladonna'
307 lp_path = '~{owner}/charms/bundles/{basket}/bundle'.format(310 basket = 'treme'
308 owner=owner, basket=basket)311 bundle_name = 'gigis'
309 lp_short_url = 'lp:{path}'.format(path=lp_path)312 rev = 1
310 self.assertIn(lp_short_url, resp.body)313 basket_with_rev = '%s/%d' % (basket, rev)
311 # Ensure the long url to the branch on Launchpad is in the body.314 bundle_1, bundle_data_1 = factory.makeBundle(
312 lp_long_url = 'https://code.launchpad.net/{path}'.format(path=lp_path)315 self.db, name=bundle_name, owner=owner,
313 self.assertIn(lp_long_url, resp.body)316 basket_with_rev=basket_with_rev, series='raring',
317 with_basket=True,
318 )
319 bundle_2, bundle_data_2 = factory.makeBundle(
320 self.db, name='other_bundle', owner=owner,
321 basket_with_rev=basket_with_rev, series='raring',
322 with_basket=True,
323 )
324 # Test that the route works.
325 path = '/bundle/~{owner}/{basket}/{bundle}'.format(
326 owner=owner, basket=basket, bundle=bundle_name)
327 resp = self.app.get(path, status=301)
328 expected = '/q/{}?type=bundle'.format(basket)
329 self.assertTrue(resp.location.endswith(expected))
330
331 def test_personal_bundle_route_not_found(self):
332 # We just forward without checking the database, so a 404 is not
333 # thrown.
334 basket = 'nobasket'
335 path = '/bundle/~noowner/{}/nobundle'.format(basket)
336 resp = self.app.get(path, status=301)
337 expected = '/q/{}?type=bundle'.format(basket)
338 self.assertTrue(resp.location.endswith(expected))
314339
315 def test_personal_bundle_route_with_revision(self):340 def test_personal_bundle_route_with_revision(self):
341 basket = 'wicker'
316 bundle, bundle_data = factory.makeBundle(342 bundle, bundle_data = factory.makeBundle(
317 self.db, name='gigis', owner='ladonna',343 self.db, name='gigis', owner='ladonna',
318 basket_with_rev='basket/1', series='raring',344 basket_with_rev='{}/1'.format(basket), series='raring',
319 )345 )
320 path = '/bundle/~ladonna/basket/1/gigis'346 path = '/bundle/~ladonna/{}/1/gigis'.format(basket)
321 response = self.app.get(path, status=200)347 resp = self.app.get(path, status=301)
322 self.assertIn(348 expected = '/q/{}?type=bundle'.format(basket)
323 'lp:~ladonna/charms/bundles/basket/bundle', response.body)349 self.assertTrue(resp.location.endswith(expected))
324350
325 def test_personal_bundle_route_404(self):351 def test_personal_bundle_route_404(self):
352 basket = 'wicker'
326 bundle, bundle_data = factory.makeBundle(353 bundle, bundle_data = factory.makeBundle(
327 self.db, name='gigis', owner='ladonna',354 self.db, name='gigis', owner='ladonna',
328 basket_with_rev='basket/1', series='raring',355 basket_with_rev='{}/1'.format(basket), series='raring',
329 )356 )
330 path = '/bundle/~ladonna/basket/1/XXX'357 path = '/bundle/~ladonna/{}/1/XXX'.format(basket)
331 self.app.get(path, status=404)358 resp = self.app.get(path, status=301)
359 expected = '/q/{}?type=bundle'.format(basket)
360 self.assertTrue(resp.location.endswith(expected))
332361
333 def test_personal_bundle_with_revision_json_route(self):362 def test_personal_bundle_with_revision_json_route(self):
334 # A JSON-encoded bundle can be requested in which the ID includes both363 # A JSON-encoded bundle can be requested in which the ID includes both
@@ -364,7 +393,7 @@
364 path = '/bundle/~ladonna/basket/1/XXX/json'393 path = '/bundle/~ladonna/basket/1/XXX/json'
365 self.app.get(path, status=404)394 self.app.get(path, status=404)
366395
367 def test_official_bundle_route(self):396 def test_official_bundle_json_route(self):
368 bundle, bundle_data = factory.makeBundle(397 bundle, bundle_data = factory.makeBundle(
369 self.db, name='wiki', owner='charmers',398 self.db, name='wiki', owner='charmers',
370 basket_with_rev='wiki-basket/1', series='raring', promulgated=True,399 basket_with_rev='wiki-basket/1', series='raring', promulgated=True,
@@ -376,14 +405,16 @@
376 self.assertIn(str(bundle.data['services']), resp.body)405 self.assertIn(str(bundle.data['services']), resp.body)
377406
378 def test_official_bundle_with_revision_route(self):407 def test_official_bundle_with_revision_route(self):
408 basket = 'wiki-basket'
379 bundle, bundle_data = factory.makeBundle(409 bundle, bundle_data = factory.makeBundle(
380 self.db, name='wiki', owner='charmers',410 self.db, name='wiki', owner='charmers',
381 basket_with_rev='wiki-basket/1', series='raring', promulgated=True,411 basket_with_rev='{}/1'.format(basket), series='raring',
412 promulgated=True,
382 )413 )
383 path = '/bundle/wiki-basket/1/wiki'414 path = '/bundle/{}/1/wiki'.format(basket)
384 response = self.app.get(path, status=200)415 resp = self.app.get(path, status=301)
385 self.assertIn(416 expected = '/q/{}?type=bundle'.format(basket)
386 'lp:~charmers/charms/bundles/wiki-basket/bundle', response.body)417 self.assertTrue(resp.location.endswith(expected))
387418
388 def test_official_bundle_route_404(self):419 def test_official_bundle_route_404(self):
389 bundle, bundle_data = factory.makeBundle(420 bundle, bundle_data = factory.makeBundle(
@@ -394,25 +425,15 @@
394 self.app.get(path, status=404)425 self.app.get(path, status=404)
395426
396 def test_bundle_short_url_route(self):427 def test_bundle_short_url_route(self):
397 bundle, bundle_data = factory.makeBundle(428 basket = 'wicker'
398 self.db, name='gigis', owner='ladonna',429 bundle, bundle_data = factory.makeBundle(
399 basket_with_rev='basket/1', series='raring', promulgated=True,430 self.db, name='gigis', owner='ladonna',
400 )431 basket_with_rev='{}/1'.format(basket), series='raring',
401 self.app.get('/' + bundle.short_url, status=200)432 promulgated=True,
402433 )
403434 resp = self.app.get('/' + bundle.short_url, status=301)
404class BundleDisplayTests(WebTestBase):435 expected = '/q/{}?type=bundle'.format(basket)
405 """Verify that bundles are displayed correctly."""436 self.assertTrue(resp.location.endswith(expected))
406
407 def test_download_statistics_are_displayed(self):
408 # The rendered page includes details about the number of times this
409 # bundle has been "downloaded" (actually, it means "deployed").
410 bundle, bundle_data = factory.makeBundle(
411 self.db, name='gigis', owner='ladonna',
412 basket_with_rev='basket/1', series='raring', promulgated=True,
413 )
414 response = self.app.get('/' + bundle.short_url, status=200)
415 self.assertIn('>Downloads<', response.body)
416437
417438
418class StatisticsTests(unittest.TestCase):439class StatisticsTests(unittest.TestCase):
419440
=== modified file 'default.ini'
--- default.ini 2015-02-09 14:16:33 +0000
+++ default.ini 2015-02-18 14:26:31 +0000
@@ -29,6 +29,9 @@
29# App Settings29# App Settings
30project_name = Charm Browser30project_name = Charm Browser
3131
32# Caching settings
33http_cache =
34
32# List of groups/teams that have edit access.35# List of groups/teams that have edit access.
33openid_teams = charmers,juju-jitsu36openid_teams = charmers,juju-jitsu
3437
@@ -71,6 +74,9 @@
71# New web site for redirection74# New web site for redirection
72redirect_jujucharms = https://jujucharms.com75redirect_jujucharms = https://jujucharms.com
7376
77# This signals that we are running under test.
78is_testing = false
79
74[server:main]80[server:main]
75use = egg:Paste#http81use = egg:Paste#http
76host = 0.0.0.082host = 0.0.0.0
@@ -134,3 +140,4 @@
134format = %(asctime)s %(message)s140format = %(asctime)s %(message)s
135141
136# End logging configuration142# End logging configuration
143

Subscribers

People subscribed via source and target branches

to all changes: