charmworld

Merge lp:~bac/charmworld/promulgate-bundles into lp:charmworld

  1. promulgate-bundles
  2. Merge into trunk
Proposed by Brad Crittenden
Status: Merged
Approved by: Brad Crittenden
Approved revision: 490
Merged at revision: 485
Proposed branch: lp:~bac/charmworld/promulgate-bundles
Merge into: lp:charmworld
Diff against target: 364 lines (+100/-44)
10 files modified
charmworld/forms/featured.py (+19/-3)
charmworld/jobs/ingest.py (+9/-5)
charmworld/jobs/lp.py (+16/-16)
charmworld/jobs/tests/test_ingest.py (+3/-3)
charmworld/jobs/tests/test_lp.py (+10/-6)
charmworld/jobs/utils.py (+1/-2)
charmworld/models.py (+8/-4)
charmworld/tests/test_forms.py (+25/-0)
charmworld/tests/test_models.py (+3/-1)
charmworld/views/featured.py (+6/-4)
To merge this branch: bzr merge lp:~bac/charmworld/promulgate-bundles
Reviewer Review Type Date Requested Status
Juju Gui Bot continuous-integration Approve
Charmworld Developers Pending
Review via email: mp+207016@code.launchpad.net

Commit message

Promulgate bundles owned by ~charmers.

https://codereview.appspot.com/65550043/

R=benji

Description of the change

Promulgate bundles owned by ~charmers.

https://codereview.appspot.com/65550043/

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Reviewers: mp+207016_code.launchpad.net,

Message:
Please take a look.

Description:
Promulgate bundles owned by ~charmers.

https://code.launchpad.net/~bac/charmworld/promulgate-bundles/+merge/207016

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/65550043/

Affected files (+83, -19 lines):
   A [revision details]
   M charmworld/forms/featured.py
   M charmworld/jobs/ingest.py
   M charmworld/jobs/lp.py
   M charmworld/jobs/tests/test_ingest.py
   M charmworld/jobs/tests/test_lp.py
   M charmworld/jobs/utils.py
   M charmworld/models.py
   A charmworld/tests/test_forms.py
   M charmworld/tests/test_models.py
   M charmworld/views/featured.py

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

https://codereview.appspot.com/65550043/diff/1/charmworld/jobs/tests/test_lp.py
File charmworld/jobs/tests/test_lp.py (right):

https://codereview.appspot.com/65550043/diff/1/charmworld/jobs/tests/test_lp.py#newcode177
charmworld/jobs/tests/test_lp.py:177: # is not used to determine whether
a bundle is promulgated.
Oops, missed this c-n-p change.

https://codereview.appspot.com/65550043/

Revision history for this message
Benji York (benji) wrote :

LGTM

https://codereview.appspot.com/65550043/diff/1/charmworld/jobs/lp.py
File charmworld/jobs/lp.py (right):

https://codereview.appspot.com/65550043/diff/1/charmworld/jobs/lp.py#newcode70
charmworld/jobs/lp.py:70:
Some prose describing the fact that the first "data['promulgated'] =" is
for bundles only and the second is for charms only would be nice. It
took me a while to figure that out.

https://codereview.appspot.com/65550043/

lp:~bac/charmworld/promulgate-bundles updated
488. By Brad Crittenden

Restructured code to be clearer wrt charm vs bundle processing.

489. By Brad Crittenden

Fixed copyright years.

490. By Brad Crittenden

Account for removed annoying log message in test.

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
1=== modified file 'charmworld/forms/featured.py'
2--- charmworld/forms/featured.py 2013-04-17 15:03:35 +0000
3+++ charmworld/forms/featured.py 2014-02-18 20:04:30 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the
6+# Copyright 2012-2014 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 __metaclass__ = type
10@@ -10,7 +10,23 @@
11 SchemaNode,
12 )
13
14+from charmworld.search import BUNDLE
15+
16+
17+def get_schema(kind):
18+ """Based on the kind return the right schema."""
19+ if kind == BUNDLE:
20+ return BundleFeaturedness()
21+ return CharmFeaturedness()
22+
23
24 class CharmFeaturedness(MappingSchema):
25- is_featured = SchemaNode(Boolean(), title='Feature this charm',
26- missing=False)
27+ is_featured = SchemaNode(
28+ Boolean(), title='Feature this charm',
29+ missing=False)
30+
31+
32+class BundleFeaturedness(MappingSchema):
33+ is_featured = SchemaNode(
34+ Boolean(), title='Feature this bundle',
35+ missing=False)
36
37=== modified file 'charmworld/jobs/ingest.py'
38--- charmworld/jobs/ingest.py 2014-01-27 19:09:48 +0000
39+++ charmworld/jobs/ingest.py 2014-02-18 20:04:30 +0000
40@@ -1,4 +1,4 @@
41-# Copyright 2013 Canonical Ltd. This software is licensed under the
42+# Copyright 2013-2014 Canonical Ltd. This software is licensed under the
43 # GNU Affero General Public License version 3 (see the file LICENSE).
44
45 import calendar
46@@ -406,10 +406,11 @@
47 super(UpdateBundleJob, self).__init__()
48
49 def store_bundles(self, deployer_config, owner, basket_id, first_change,
50- last_change, changes, branch_spec):
51+ last_change, changes, branch_spec, promulgated):
52 store_bundles(
53 self.db.bundles, deployer_config, owner, basket_id, first_change,
54- last_change, changes, branch_spec)
55+ last_change, changes, branch_spec,
56+ promulgated=promulgated)
57
58 @staticmethod
59 def set_basket_info(data, revno):
60@@ -474,8 +475,11 @@
61 self.store_bundles(
62 deployer_config, basket_data['owner'],
63 basket_data['name_revno'],
64- basket_data['first_change'], basket_data['last_change'],
65- basket_data['changes'], basket_data['branch_spec'])
66+ basket_data['first_change'],
67+ basket_data['last_change'],
68+ basket_data['changes'],
69+ basket_data['branch_spec'],
70+ basket_data['promulgated'])
71 self.update_ingest_status()
72
73
74
75=== modified file 'charmworld/jobs/lp.py'
76--- charmworld/jobs/lp.py 2014-01-22 15:27:39 +0000
77+++ charmworld/jobs/lp.py 2014-02-18 20:04:30 +0000
78@@ -1,4 +1,4 @@
79-# Copyright 2012, 2013 Marco Ceppi, Canonical Ltd. This software is
80+# Copyright 2012-2014 Marco Ceppi, Canonical Ltd. This software is
81 # licensed under the GNU Affero General Public License version 3 (see
82 # the file LICENSE).
83
84@@ -46,33 +46,33 @@
85 return logging.getLogger('charm.launchpad')
86
87
88-def all_branch_data(charm_data=None):
89+def all_branch_data(branch_data=None):
90 # Return data needed for the ingest job for charm branches on Launchpad.
91- log = getLogger()
92 charms = []
93 baskets = []
94- if not charm_data:
95- # Get all charm tip branches from Launchpad.
96- charm_data = get_branch_tips()
97+ if branch_data is None:
98+ # Get all charm and bundle tip branches from Launchpad.
99+ branch_data = get_branch_tips()
100
101- for repo, commit, series in charm_data:
102+ for repo, commit, series in branch_data:
103 _, branch_name = repo.rsplit('/', 1)
104
105 data = parse_branch(repo, series, commit)
106
107- data['promulgated'] = data['series'] in data['distro_series']
108-
109 if data['series'] == BASKET_SERIES:
110+ # The branch is a basket of bundles.
111+ data['promulgated'] = data['owner'] == u'charmers'
112 if data['bname'] == 'bundle':
113 baskets.append(data)
114- continue
115+ else:
116+ # The branch is a charm.
117+ data['promulgated'] = data['series'] in data['distro_series']
118
119- if data['bname'] != 'trunk':
120- log.debug('Skipped branch %s', repo)
121- continue
122- # The branch is a charm trunk branch. Use it.
123- data['branch_deleted'] = False
124- charms.append(data)
125+ if data['bname'] != 'trunk':
126+ continue
127+ # The branch is a charm trunk branch. Use it.
128+ data['branch_deleted'] = False
129+ charms.append(data)
130
131 return charms, baskets
132
133
134=== modified file 'charmworld/jobs/tests/test_ingest.py'
135--- charmworld/jobs/tests/test_ingest.py 2014-01-27 19:09:48 +0000
136+++ charmworld/jobs/tests/test_ingest.py 2014-02-18 20:04:30 +0000
137@@ -1,4 +1,4 @@
138-# Copyright 2013 Canonical Ltd. This software is licensed under the
139+# Copyright 2013-2014 Canonical Ltd. This software is licensed under the
140 # GNU Affero General Public License version 3 (see the file LICENSE).
141
142 __metaclass__ = type
143@@ -595,7 +595,7 @@
144 fs.put(deployer_config, _id=DEPLOYER_CONFIG_HASH)
145 job.store_bundles(
146 yaml.safe_load(deployer_config), basket_data['owner'],
147- basket_data['name_revno'], None, None, None, None)
148+ basket_data['name_revno'], None, None, None, None, False)
149 self.assertIsNotNone(self.db.bundles.find_one(bundle_id))
150
151 def test_job_run_stores_bundles_in_the_db(self):
152@@ -633,7 +633,7 @@
153 job.run(basket_data)
154 store_bundles.assert_called_with(
155 {'foo': {}}, 'charmers', 'dummy', None, None, None,
156- None)
157+ None, False)
158
159 def test_proof_is_called_with_branch_dir(self):
160 basket_data = factory.get_payload_json(name='wordpress')
161
162=== modified file 'charmworld/jobs/tests/test_lp.py'
163--- charmworld/jobs/tests/test_lp.py 2014-01-22 14:27:54 +0000
164+++ charmworld/jobs/tests/test_lp.py 2014-02-18 20:04:30 +0000
165@@ -1,4 +1,4 @@
166-# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the
167+# Copyright 2012-2014 Canonical Ltd. This software is licensed under the
168 # GNU Affero General Public License version 3 (see the file LICENSE).
169
170 from argparse import Namespace
171@@ -124,14 +124,10 @@
172 def test_all_branch_data_not_trunk(self):
173 # If the branch is not trunk, the charm is not returned by
174 # all_branch_data.
175- handler = self.get_handler("charm.launchpad")
176 charm_data = [[u'~charmers/charms/precise/someproject/foo',
177 u'ja@appflower.com-20120329093714-s2m9e28dwotmijqc',
178 []]]
179 charms = all_branch_data(charm_data)[0]
180- log_messages = [record.getMessage() for record in handler.buffer]
181- err_msg = ("Skipped branch ~charmers/charms/precise/someproject/foo")
182- self.assertIn(err_msg, log_messages)
183 self.assertEqual([], charms)
184
185 def test_all_charms_limit(self):
186@@ -165,9 +161,17 @@
187 self.assertEqual([], charms)
188 self.assertEqual([], baskets)
189
190- def test_promulgated_set_for_bundles(self):
191+ def test_promulgated_not_set_for_bundles_on_series(self):
192+ # The data from Launchpad does not have the series for bundles, so it
193+ # is not used to determine whether a bundle is promulgated.
194 branch = '~foo/charms/' + BASKET_SERIES + '/bar/bundle'
195 charms, baskets = all_branch_data([(branch, 'asdf', [BASKET_SERIES])])
196+ self.assertFalse(baskets[0]['promulgated'])
197+
198+ def test_promulgated_set_for_bundles_for_charmers(self):
199+ # The branch is owned by ~charmers so it is marked as promulgated.
200+ branch = '~charmers/charms/' + BASKET_SERIES + '/bar/bundle'
201+ charms, baskets = all_branch_data([(branch, 'asdf', [])])
202 self.assertTrue(baskets[0]['promulgated'])
203
204 def test_db_charms(self):
205
206=== modified file 'charmworld/jobs/utils.py'
207--- charmworld/jobs/utils.py 2013-07-19 19:32:28 +0000
208+++ charmworld/jobs/utils.py 2014-02-18 20:04:30 +0000
209@@ -1,4 +1,4 @@
210-# Copyright 2012, 2013 Marco Ceppi, Canonical Ltd. This software is
211+# Copyright 2012-2014 Marco Ceppi, Canonical Ltd. This software is
212 # licensed under the GNU Affero General Public License version 3 (see
213 # the file LICENSE).
214
215@@ -34,7 +34,6 @@
216
217 Returned data is augmented with commit if present
218 """
219-
220 data = {}
221
222 parts = branch_spec.split("/")
223
224=== modified file 'charmworld/models.py'
225--- charmworld/models.py 2014-02-10 21:29:09 +0000
226+++ charmworld/models.py 2014-02-18 20:04:30 +0000
227@@ -1,4 +1,4 @@
228-# Copyright 2012, 2013 Marco Ceppi, Canonical Ltd. This software is
229+# Copyright 2012-2014 Marco Ceppi, Canonical Ltd. This software is
230 # licensed under the GNU Affero General Public License version 3 (see
231 # the file LICENSE).
232
233@@ -1795,7 +1795,7 @@
234
235
236 def make_bundle_doc(data, owner, basket_id, bundle_name, first_change,
237- last_change, changes, branch_spec):
238+ last_change, changes, branch_spec, promulgated=False):
239 basket_name, basket_revision = basket_id.split('/')
240 _id = Bundle.construct_id(owner, basket_name, bundle_name, basket_revision)
241 return {
242@@ -1809,11 +1809,13 @@
243 'last_change': last_change,
244 'changes': changes,
245 'branch_spec': branch_spec,
246+ 'promulgated': promulgated,
247 }
248
249
250 def store_bundles(collection, deployer_config, owner, basket_id, first_change,
251- last_change, changes, branch_spec, index_client=None):
252+ last_change, changes, branch_spec, index_client=None,
253+ promulgated=False):
254 """Store a basket of bundles into MongoDB and/or ElasticSearch.
255
256 :param: collection: A db bundles collection. If None the bundles are not
257@@ -1825,6 +1827,8 @@
258 :param: index_client: The elastic search client. If None it will be
259 retrieved. Useful for Mongo-only tests where the indexing is not
260 needed.
261+ :param: promulgated: Is this bundle promulgated, blessed by the charmers
262+ and eligible to be featured?
263 """
264
265 if index_client is None:
266@@ -1837,7 +1841,7 @@
267 data = get_flattened_deployment(deployer_config, bundle_name)
268 index_data[bundle_name] = make_bundle_doc(
269 data, owner, basket_id, bundle_name, first_change, last_change,
270- changes, branch_spec)
271+ changes, branch_spec, promulgated)
272 if collection is not None:
273 collection.save(index_data[bundle_name])
274
275
276=== added file 'charmworld/tests/test_forms.py'
277--- charmworld/tests/test_forms.py 1970-01-01 00:00:00 +0000
278+++ charmworld/tests/test_forms.py 2014-02-18 20:04:30 +0000
279@@ -0,0 +1,25 @@
280+# Copyright 2014 Canonical Ltd. This software is licensed under the GNU
281+# Affero General Public License version 3 (see the file LICENSE.txt).
282+
283+from charmworld.forms.featured import (
284+ get_schema,
285+)
286+from charmworld.search import (
287+ BUNDLE,
288+ CHARM,
289+)
290+
291+from unittest import TestCase
292+
293+
294+class TestGetSchema(TestCase):
295+
296+ def test_CharmFeaturedness(self):
297+ schema = get_schema(CHARM)
298+ self.assertIn(
299+ 'Feature this charm', schema.children[0].title)
300+
301+ def test_BundleFeaturedness(self):
302+ schema = get_schema(BUNDLE)
303+ self.assertIn(
304+ 'Feature this bundle', schema.children[0].title)
305
306=== modified file 'charmworld/tests/test_models.py'
307--- charmworld/tests/test_models.py 2014-02-11 15:37:54 +0000
308+++ charmworld/tests/test_models.py 2014-02-18 20:04:30 +0000
309@@ -1667,6 +1667,7 @@
310 'last_change': None,
311 'changes': None,
312 'branch_spec': None,
313+ 'promulgated': False,
314 },
315 self.db.bundles.find_one(_id))
316
317@@ -2075,7 +2076,7 @@
318 def test_bundle_doc(self):
319 doc = make_bundle_doc(
320 {'a': 'b'}, 'foo', 'bar/9', 'baz', None, None, None,
321- 'fake/branch/spec')
322+ 'fake/branch/spec', promulgated=True)
323 self.assertEqual({
324 'owner': 'foo',
325 'basket_name': 'bar',
326@@ -2087,6 +2088,7 @@
327 'last_change': None,
328 'changes': None,
329 'branch_spec': 'fake/branch/spec',
330+ 'promulgated': True,
331 }, doc)
332
333
334
335=== modified file 'charmworld/views/featured.py'
336--- charmworld/views/featured.py 2013-09-04 16:51:14 +0000
337+++ charmworld/views/featured.py 2014-02-18 20:04:30 +0000
338@@ -1,4 +1,4 @@
339-# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the
340+# Copyright 2012-2014 Canonical Ltd. This software is licensed under the
341 # GNU Affero General Public License version 3 (see the file LICENSE).
342
343 from deform import (
344@@ -8,8 +8,10 @@
345 from pyramid.httpexceptions import HTTPFound
346 from pyramid.view import view_config
347
348-from charmworld.forms.featured import CharmFeaturedness
349-from charmworld.models import FeaturedSource
350+from charmworld.forms.featured import get_schema
351+from charmworld.models import (
352+ FeaturedSource,
353+)
354 from charmworld.views.helpers import (
355 find_charm,
356 find_bundle,
357@@ -19,7 +21,7 @@
358
359 def featured_edit(request, item, kind, url):
360 """Back end for featuring an "item" which can be a charm or bundle."""
361- form = Form(CharmFeaturedness(), buttons=('submit',))
362+ form = Form(get_schema(kind), buttons=('submit',))
363 form.css_class = 'form-horizontal well'
364 featured_source = FeaturedSource.from_db(request.db)
365

Subscribers

People subscribed via source and target branches

to all changes: