Merge lp:~benji/charmworld/expose-bundle-changes into lp:~juju-jitsu/charmworld/trunk

Proposed by Benji York
Status: Merged
Merged at revision: 427
Proposed branch: lp:~benji/charmworld/expose-bundle-changes
Merge into: lp:~juju-jitsu/charmworld/trunk
Diff against target: 445 lines (+122/-69)
10 files modified
charmworld/jobs/ingest.py (+33/-23)
charmworld/jobs/tests/test_bzr.py (+51/-22)
charmworld/jobs/tests/test_ingest.py (+3/-3)
charmworld/migrations/versions/tests/test_migrations.py (+1/-0)
charmworld/models.py (+10/-5)
charmworld/templates/bundle.pt (+0/-8)
charmworld/testing/factory.py (+2/-1)
charmworld/tests/test_models.py (+15/-4)
charmworld/tests/test_search.py (+3/-2)
charmworld/views/tests/test_api.py (+4/-1)
To merge this branch: bzr merge lp:~benji/charmworld/expose-bundle-changes
Reviewer Review Type Date Requested Status
Juju-Jitsu Hackers Pending
Review via email: mp+192389@code.launchpad.net

Description of the change

Add recent bzr changes to bundles.

https://codereview.appspot.com/15100046/

To post a comment you must log in.
Brad Crittenden (bac) wrote :

LGTM

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

https://codereview.appspot.com/15100046/diff/1/charmworld/jobs/ingest.py#newcode424
charmworld/jobs/ingest.py:424: if revision.timestamp < since:
So since cannot be None?

https://codereview.appspot.com/15100046/diff/1/charmworld/jobs/ingest.py#newcode424
charmworld/jobs/ingest.py:424: if revision.timestamp < since:
So we're sure since can never be None?

https://codereview.appspot.com/15100046/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmworld/jobs/ingest.py'
2--- charmworld/jobs/ingest.py 2013-10-17 21:32:57 +0000
3+++ charmworld/jobs/ingest.py 2013-10-23 18:29:57 +0000
4@@ -261,7 +261,7 @@
5 update_download_count(store, charm_data)
6 update_proof_data(charm_data, log)
7 update_jenkins_data(db, charm_data, log)
8- update_from_revisions(charm_data, newest_revision=revision)
9+ update_revision_details(charm_data, newest_revision=revision)
10 update_date_created(charm_data, log)
11 scan_charm(charm_data, fs, log)
12 update_hash(charm_data)
13@@ -309,8 +309,11 @@
14 self.working_dir = working_dir
15 super(UpdateBundleJob, self).__init__()
16
17- def store_bundles(self, deployer_config, owner, basket_id):
18- store_bundles(self.db.bundles, deployer_config, owner, basket_id)
19+ def store_bundles(self, deployer_config, owner, basket_id, first_change,
20+ last_change, changes):
21+ store_bundles(
22+ self.db.bundles, deployer_config, owner, basket_id, first_change,
23+ last_change, changes)
24
25 @staticmethod
26 def set_basket_info(data, revno):
27@@ -319,6 +322,9 @@
28 def decorate_basket(self, basket_data, fs):
29 branch_dir = fetch_branch(self.working_dir, basket_data, self.log,
30 revisionId=basket_data['commit'])
31+ basket_data['branch_dir'] = branch_dir
32+ update_revision_details(
33+ basket_data, newest_revision=basket_data['commit'])
34 branch = Branch.open(branch_dir)
35 revno = branch.revision_id_to_revno(basket_data['commit'])
36 self.set_basket_info(basket_data, revno)
37@@ -340,7 +346,8 @@
38 self.db.baskets.save(basket_data)
39 deployer_config = self.get_deployer_config(fs, basket_data)
40 self.store_bundles(
41- deployer_config, basket_data['owner'], basket_data['name_revno'])
42+ deployer_config, basket_data['owner'], basket_data['name_revno'],
43+ None, None, None)
44
45
46 def _rev_info(r, branch):
47@@ -354,27 +361,34 @@
48 return d
49
50
51-def update_from_revisions(charm_data, limit=10, since=None,
52- newest_revision=None):
53- if charm_data['branch_deleted']:
54+def find_revision_cutoff(now=None, settings=settings):
55+ days_of_revisions = settings.get('days_of_revisions')
56+ if days_of_revisions is None:
57+ days_of_revisions = 30
58+ if now is None:
59+ now = datetime.utcnow()
60+ cutoff = now - timedelta(int(days_of_revisions))
61+ return calendar.timegm(cutoff.timetuple())
62+
63+
64+def update_revision_details(data, limit=10, newest_revision=None):
65+ # If the branch has been deleted, there can be no new revisions to pull in.
66+ if data['branch_deleted']:
67 return
68- if since is None:
69- days_of_revisions = settings.get('days_of_revisions')
70- if days_of_revisions is not None:
71- cutoff = datetime.utcnow() - timedelta(int(days_of_revisions))
72- since = calendar.timegm(cutoff.timetuple())
73- branch_dir = charm_data['branch_dir']
74- changes = get_changes(branch_dir, limit, since, newest_revision)
75- charm_data.update(changes)
76+ since = find_revision_cutoff()
77+ changes, first_change, last_change = get_changes(
78+ data['branch_dir'], limit, since, newest_revision)
79+ data['changes'] = changes
80+ data['first_change'] = first_change
81+ data['last_change'] = last_change
82
83
84 def get_changes(branch_dir, limit, since, newest_revision=None):
85- charm_data = {}
86 branch = Branch.open(branch_dir)
87 branch.lock_read()
88 try:
89 revisions = get_revisions(branch, limit, since)
90- charm_data['changes'] = changes = []
91+ changes = []
92 # If there is no limit on how new a revision we can include, then
93 # consider the starting revision to be already found.
94 found_start = newest_revision is None
95@@ -391,11 +405,7 @@
96 last_change = changes[0]
97 first = branch.repository.get_revision(branch.get_rev_id(1))
98 first_change = _rev_info(first, branch)
99- charm_data.update({
100- 'last_change': last_change,
101- 'first_change': first_change,
102- })
103- return charm_data
104+ return changes, first_change, last_change
105 finally:
106 branch.unlock()
107
108@@ -411,7 +421,7 @@
109 break
110 revision = branch.repository.get_revision(revision_id)
111 if num >= limit:
112- if since is None or revision.timestamp < since:
113+ if revision.timestamp < since:
114 break
115 revs.append(revision)
116 return revs
117
118=== modified file 'charmworld/jobs/tests/test_bzr.py'
119--- charmworld/jobs/tests/test_bzr.py 2013-10-17 20:19:55 +0000
120+++ charmworld/jobs/tests/test_bzr.py 2013-10-23 18:29:57 +0000
121@@ -3,6 +3,7 @@
122
123
124 from contextlib import contextmanager
125+import datetime
126 from logging import getLogger
127 import os
128 import time
129@@ -13,13 +14,14 @@
130
131 from charmworld.jobs import ingest
132 from charmworld.jobs.ingest import (
133+ _rev_info,
134 add_files,
135 checkout_branch,
136 do_bzr_update,
137+ find_revision_cutoff,
138 get_changes,
139 get_revisions,
140- _rev_info,
141- update_from_revisions,
142+ update_revision_details,
143 )
144 from charmworld.models import getfs
145 from charmworld.testing import (
146@@ -268,36 +270,63 @@
147 self.assertEqual([YRH, ZRH], rev_info['authors'])
148 self.assertEqual(JRH, rev_info['committer'])
149
150- def test_get_changes(self):
151+ def test_get_changes_with_one_revision(self):
152+ tree = self.make_locked_tree()
153+ self.num_commit(tree, 1)
154+ changes, first_change, last_change = get_changes(
155+ 'tree', 10, time.time())
156+ self.assertEqual(first_change, last_change)
157+ self.assertEqual([first_change], changes)
158+
159+ def test_get_changes_with_multiple_revisions(self):
160 tree = self.make_locked_tree()
161 first_id = self.num_commit(tree, 1)
162- charm_data = get_changes('tree', 10, time.time())
163- self.assertItemsEqual(['changes', 'first_change', 'last_change'],
164- charm_data.keys())
165- self.assertEqual(charm_data['first_change'], charm_data['last_change'])
166- self.assertEqual([charm_data['first_change']], charm_data['changes'])
167 for num in range(1, 11):
168 self.num_commit(tree, 1)
169- charm_data = get_changes('tree', 10, time.time())
170- self.assertNotEqual(charm_data['first_change'],
171- charm_data['last_change'])
172- self.assertEqual(charm_data['last_change'], charm_data['changes'][0])
173- self.assertNotEqual(charm_data['first_change'],
174- charm_data['changes'][-1])
175- self.assertEqual(self.get_rev_info(tree.branch, first_id),
176- charm_data['first_change'])
177+ changes, first_change, last_change = get_changes(
178+ 'tree', 10, time.time())
179+ self.assertNotEqual(first_change, last_change)
180+ self.assertEqual(last_change, changes[0])
181+ self.assertNotEqual(first_change, changes[-1])
182+ self.assertEqual(
183+ self.get_rev_info(tree.branch, first_id),
184+ first_change)
185
186 def test_get_changes_no_revisions(self):
187 self.make_locked_tree()
188- charm_data = get_changes('tree', 10, time.time())
189- self.assertEqual([], charm_data['changes'])
190- self.assertIs(None, charm_data['first_change'])
191- self.assertIs(None, charm_data['last_change'])
192+ changes, first_change, last_change = get_changes(
193+ 'tree', 10, time.time())
194+ self.assertEqual([], changes)
195+ self.assertIs(None, first_change)
196+ self.assertIs(None, last_change)
197
198 def test_branch_deleted(self):
199- # update_from_revisions does not do anything if the Launchpad branch
200+ # update_revision_details does not do anything if the Launchpad branch
201 # of a charm is deleted.
202 charm = factory.get_charm_json(branch_deleted=True)
203 with patch.object(ingest, 'get_changes') as mock:
204- update_from_revisions(charm)
205+ update_revision_details(charm)
206 self.assertFalse(mock.called)
207+
208+
209+class TestFindRevisionCutoff(TestCase):
210+ """Tests for charmworld.jobs.ingest.find_revision_cutoff."""
211+
212+ def test_return_type(self):
213+ # find_revision_cutoff returns seconds since the epoch (an int).
214+ self.assertEqual(type(find_revision_cutoff()), int)
215+
216+ def test_days_of_revisions_is_respected(self):
217+ # If the cutoff date is further in the past, the resulting seconds
218+ # since the epoch is further in the past too.
219+ old = find_revision_cutoff(settings=dict(days_of_revisions=10))
220+ older = find_revision_cutoff(settings=dict(days_of_revisions=20))
221+ self.assertGreater(old, older)
222+
223+ def test_days_are_the_right_size(self):
224+ # If we start at time 0 and ask for one day in the past, we get the
225+ # obvious value.
226+ time = find_revision_cutoff(
227+ now=datetime.datetime.utcfromtimestamp(0),
228+ settings=dict(days_of_revisions=1))
229+ self.assertEqual(time, -86400)
230
231=== modified file 'charmworld/jobs/tests/test_ingest.py'
232--- charmworld/jobs/tests/test_ingest.py 2013-10-17 21:32:57 +0000
233+++ charmworld/jobs/tests/test_ingest.py 2013-10-23 18:29:57 +0000
234@@ -533,8 +533,8 @@
235 """)
236 fs.put(deployer_config, _id=DEPLOYER_CONFIG_HASH)
237 job.store_bundles(
238- yaml.safe_load(deployer_config),
239- basket_data['owner'], basket_data['name_revno'])
240+ yaml.safe_load(deployer_config), basket_data['owner'],
241+ basket_data['name_revno'], None, None, None)
242 self.assertIsNotNone(self.db.bundles.find_one(bundle_id))
243
244 def test_job_run_stores_bundles_in_the_db(self):
245@@ -559,7 +559,7 @@
246 with patch.object(job, 'store_bundles') as store_bundles:
247 job.run(basket_data)
248 store_bundles.assert_called_with(
249- {'foo': {}}, 'charmers', 'dummy')
250+ {'foo': {}}, 'charmers', 'dummy', None, None, None)
251
252
253 class TestUpdateCharm(MongoTestBase):
254
255=== modified file 'charmworld/migrations/versions/tests/test_migrations.py'
256--- charmworld/migrations/versions/tests/test_migrations.py 2013-09-23 15:58:47 +0000
257+++ charmworld/migrations/versions/tests/test_migrations.py 2013-10-23 18:29:57 +0000
258@@ -60,6 +60,7 @@
259 owner, basket_name, bundle_name)
260 store_bundles(
261 self.db.bundles, parsed, owner, basket_id,
262+ None, None, None,
263 index_client=self.index_client)
264
265 def test_bundles_are_removed_from_elastic_search(self):
266
267=== modified file 'charmworld/models.py'
268--- charmworld/models.py 2013-10-22 22:57:51 +0000
269+++ charmworld/models.py 2013-10-23 18:29:57 +0000
270@@ -1743,7 +1743,8 @@
271 return options
272
273
274-def make_bundle_doc(data, owner, basket_id, bundle_name):
275+def make_bundle_doc(data, owner, basket_id, bundle_name, first_change,
276+ last_change, changes):
277 basket_name, basket_revision = basket_id.split('/')
278 _id = Bundle.construct_id(owner, basket_name, bundle_name, basket_revision)
279 return {
280@@ -1753,11 +1754,14 @@
281 'basket_name': basket_name,
282 'basket_revision': int(basket_revision),
283 'data': data,
284+ 'first_change': first_change,
285+ 'last_change': last_change,
286+ 'changes': changes,
287 }
288
289
290-def store_bundles(collection, deployer_config, owner, basket_id,
291- index_client=None):
292+def store_bundles(collection, deployer_config, owner, basket_id, first_change,
293+ last_change, changes, index_client=None):
294 """Store a basket of bundles into MongoDB and/or ElasticSearch.
295
296 :param: collection: A db bundles collection. If None the bundles are not
297@@ -1779,8 +1783,9 @@
298 for bundle_name in deployer_config:
299 # Set up indexing.
300 data = get_flattened_deployment(deployer_config, bundle_name)
301- index_data[bundle_name] = make_bundle_doc(data, owner, basket_id,
302- bundle_name)
303+ index_data[bundle_name] = make_bundle_doc(
304+ data, owner, basket_id, bundle_name, first_change, last_change,
305+ changes)
306 if collection is not None:
307 collection.save(index_data[bundle_name])
308
309
310=== modified file 'charmworld/templates/bundle.pt'
311--- charmworld/templates/bundle.pt 2013-10-11 13:51:05 +0000
312+++ charmworld/templates/bundle.pt 2013-10-23 18:29:57 +0000
313@@ -103,14 +103,6 @@
314 </li>
315 </ul>
316 </div>
317- <tal:comment condition="nothing">
318- <div tal:condition="is_owner">
319- You are the owner. In the future we'll show you extra stuff here.
320- </div>
321- <div tal:condition="not: is_owner">
322- If you were the author/owner you'd see extra stuff here.
323- </div>
324- </tal:comment>
325 </metal:block>
326 </body>
327 </html>
328
329=== modified file 'charmworld/testing/factory.py'
330--- charmworld/testing/factory.py 2013-10-04 19:45:06 +0000
331+++ charmworld/testing/factory.py 2013-10-23 18:29:57 +0000
332@@ -331,7 +331,8 @@
333 data = dict(series=series,
334 relations=relations,
335 services=services)
336- bundle_doc = make_bundle_doc(data, owner, basket_with_rev, name)
337+ bundle_doc = make_bundle_doc(
338+ data, owner, basket_with_rev, name, None, None, None)
339 bundle_doc.update(dict(branch_deleted=branch_deleted,
340 data=data,
341 description=description,
342
343=== modified file 'charmworld/tests/test_models.py'
344--- charmworld/tests/test_models.py 2013-10-22 22:57:51 +0000
345+++ charmworld/tests/test_models.py 2013-10-23 18:29:57 +0000
346@@ -1595,7 +1595,7 @@
347 basket_id = "%s/%d" % (basket_name, basket_rev)
348 _id = Bundle.construct_id(owner, basket_name, bundle_name, basket_rev)
349 store_bundles(
350- self.db.bundles, parsed, 'bac', basket_id)
351+ self.db.bundles, parsed, 'bac', basket_id, None, None, None)
352 self.assertEqual(
353 {
354 '_id': _id,
355@@ -1604,6 +1604,9 @@
356 'name': bundle_name,
357 'owner': owner,
358 'data': parsed['wordpress-stage'],
359+ 'first_change': None,
360+ 'last_change': None,
361+ 'changes': None,
362 },
363 self.db.bundles.find_one(_id))
364
365@@ -1638,7 +1641,9 @@
366
367 with patch('charmworld.models.get_flattened_deployment',
368 get_flattened_deployment):
369- store_bundles(self.db.bundles, parsed, 'bac', 'wordpress-basket/5')
370+ store_bundles(
371+ self.db.bundles, parsed, 'bac', 'wordpress-basket/5', None,
372+ None, None)
373 self.assertItemsEqual(['wordpress-stage', 'wordpress-prod'], keys)
374
375 def test_storing_a_bundle_includes_indexing_it(self):
376@@ -1656,7 +1661,9 @@
377 with patch(
378 'charmworld.models.ElasticSearchClient',
379 FauxElasticSearchClient):
380- store_bundles(self.db.bundles, {}, 'owner', 'wordpress-basket/5')
381+ store_bundles(
382+ self.db.bundles, {}, 'owner', 'wordpress-basket/5', None,
383+ None, None)
384
385 self.assertTrue(FauxElasticSearchClient.index_bundles_called)
386
387@@ -2002,7 +2009,8 @@
388 class TestMakeBundleDoc(TestCase):
389
390 def test_bundle_doc(self):
391- doc = make_bundle_doc({'a': 'b'}, 'foo', 'bar/9', 'baz')
392+ doc = make_bundle_doc(
393+ {'a': 'b'}, 'foo', 'bar/9', 'baz', None, None, None)
394 self.assertEqual({
395 'owner': 'foo',
396 'basket_name': 'bar',
397@@ -2010,6 +2018,9 @@
398 'name': 'baz',
399 'data': {'a': 'b'},
400 '_id': '~foo/bar/9/baz',
401+ 'first_change': None,
402+ 'last_change': None,
403+ 'changes': None,
404 }, doc)
405
406
407
408=== modified file 'charmworld/tests/test_search.py'
409--- charmworld/tests/test_search.py 2013-09-16 21:14:06 +0000
410+++ charmworld/tests/test_search.py 2013-10-23 18:29:57 +0000
411@@ -166,8 +166,9 @@
412 charm: cs:precise/mysql
413 """)
414 parsed = yaml.safe_load(deployer_config)
415- store_bundles(None, parsed, 'abentley', 'wordpress-basket/5',
416- index_client=self.index_client)
417+ store_bundles(
418+ None, parsed, 'abentley', 'wordpress-basket/5', None, None, None,
419+ index_client=self.index_client)
420 return _id
421
422 def test_store_bundles_bundle_name_indexed(self):
423
424=== modified file 'charmworld/views/tests/test_api.py'
425--- charmworld/views/tests/test_api.py 2013-10-22 21:02:10 +0000
426+++ charmworld/views/tests/test_api.py 2013-10-23 18:29:57 +0000
427@@ -732,6 +732,9 @@
428 description='',
429 promulgated=False,
430 branch_deleted=False,
431+ first_change=None,
432+ last_change=None,
433+ changes=None,
434 )
435 self.assertEqual(expected, bundle._representation)
436
437@@ -858,7 +861,7 @@
438 self.assertEqual(u'text/plain', response.content_type)
439
440 def test_bundle_icon(self):
441- # The current design is that all bundles, regardless of their id, will
442+ # The current design is that all bundles, regardless of their ID, will
443 # get the default bundle icon.
444 bundle = self.makeBundle()
445 response = self.get_response(

Subscribers

People subscribed via source and target branches