Merge lp:~abentley/charmworld/remove-redudant-id into lp:~juju-jitsu/charmworld/trunk

Proposed by Aaron Bentley
Status: Merged
Approved by: Aaron Bentley
Approved revision: 354
Merged at revision: 354
Proposed branch: lp:~abentley/charmworld/remove-redudant-id
Merge into: lp:~juju-jitsu/charmworld/trunk
Diff against target: 73 lines (+16/-15)
2 files modified
charmworld/jobs/ingest.py (+0/-7)
charmworld/jobs/tests/test_ingest.py (+16/-8)
To merge this branch: bzr merge lp:~abentley/charmworld/remove-redudant-id
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+180642@code.launchpad.net

Commit message

_id is not added to bundle's data attribute while ingesting.

Description of the change

Fix bug #1208481: bundles contain redundant _id in their data

Currently, _id is added to a bundle's data attribute, but this is wrong-- _id should be stored at the root level of the bundle, and already is. So this code can be deleted.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'charmworld/jobs/ingest.py'
--- charmworld/jobs/ingest.py 2013-08-13 14:46:22 +0000
+++ charmworld/jobs/ingest.py 2013-08-16 19:30:25 +0000
@@ -298,12 +298,6 @@
298 data['name_revno'] = "%s/%d" % (data['name'], revno)298 data['name_revno'] = "%s/%d" % (data['name'], revno)
299 data['_id'] = '%s/%s' % (owner_segment, data['name_revno'])299 data['_id'] = '%s/%s' % (owner_segment, data['name_revno'])
300300
301 @staticmethod
302 def decorate_bundles(basket_contents, basket_id):
303 # Give each bundle a unique ID.
304 for bundle_name, bundle in basket_contents.items():
305 bundle['_id'] = '%s/%s' % (basket_id, bundle_name)
306
307 def decorate_basket(self, basket_data, fs):301 def decorate_basket(self, basket_data, fs):
308 branch_dir = fetch_branch(self.working_dir, basket_data, self.log)302 branch_dir = fetch_branch(self.working_dir, basket_data, self.log)
309 branch = Branch.open(branch_dir)303 branch = Branch.open(branch_dir)
@@ -325,7 +319,6 @@
325 self.decorate_basket(basket_data, fs)319 self.decorate_basket(basket_data, fs)
326 self.db.baskets.save(basket_data)320 self.db.baskets.save(basket_data)
327 deployer_config = self.get_deployer_config(fs, basket_data)321 deployer_config = self.get_deployer_config(fs, basket_data)
328 self.decorate_bundles(deployer_config, basket_data['_id'])
329 self.store_bundles(322 self.store_bundles(
330 deployer_config, basket_data['owner'], basket_data['name_revno'])323 deployer_config, basket_data['owner'], basket_data['name_revno'])
331324
332325
=== modified file 'charmworld/jobs/tests/test_ingest.py'
--- charmworld/jobs/tests/test_ingest.py 2013-08-14 15:12:04 +0000
+++ charmworld/jobs/tests/test_ingest.py 2013-08-16 19:30:25 +0000
@@ -264,10 +264,10 @@
264 def test_run_updates_mongo(self):264 def test_run_updates_mongo(self):
265 # Running a bundle job creates a bundle document in mongo.265 # Running a bundle job creates a bundle document in mongo.
266 NAME = 'our-source-package'266 NAME = 'our-source-package'
267 bundle_data = factory.get_payload_json(name=NAME)267 basket_data = factory.get_payload_json(name=NAME)
268 with bundle_update_environment():268 with bundle_update_environment():
269 run_job(UpdateBundleJob(), bundle_data, db=self.db)269 run_job(UpdateBundleJob(), basket_data, db=self.db)
270 self.assertIsNotNone(self.db.baskets.find_one(bundle_data['_id']))270 self.assertIsNotNone(self.db.baskets.find_one(basket_data['_id']))
271271
272 def test_run_creates_branch(self):272 def test_run_creates_branch(self):
273 # The ingest job fetches the target branch.273 # The ingest job fetches the target branch.
@@ -399,11 +399,6 @@
399 self.assertIsNot(None, bundles_hash)399 self.assertIsNot(None, bundles_hash)
400 self.assertEqual('{foo: {}}', fs.get(bundles_hash).read())400 self.assertEqual('{foo: {}}', fs.get(bundles_hash).read())
401401
402 def test_decorate_bundles(self):
403 basket_contents = {'foo': {}}
404 UpdateBundleJob.decorate_bundles(basket_contents, '~bar/baz')
405 self.assertEqual({'foo': {'_id': '~bar/baz/foo'}}, basket_contents)
406
407 def test_missing_revisions_generate_errors(self):402 def test_missing_revisions_generate_errors(self):
408 job = UpdateBundleJob()403 job = UpdateBundleJob()
409 job.setup(self.db)404 job.setup(self.db)
@@ -472,6 +467,19 @@
472 job.run(basket_data)467 job.run(basket_data)
473 self.assertTrue(store_bundles.called)468 self.assertTrue(store_bundles.called)
474469
470 def test_job_run_does_not_alter_bundle_data(self):
471 basket_data = factory.get_payload_json(name='wordpress')
472 basket_data['name_revno'] = 'dummy'
473 job = UpdateBundleJob()
474 job.setup(self.db)
475 with patch.object(job, 'get_deployer_config',
476 side_effect=lambda x, y: {'foo': {}}):
477 with patch.object(job, 'decorate_basket'):
478 with patch.object(job, 'store_bundles') as store_bundles:
479 job.run(basket_data)
480 store_bundles.assert_called_with(
481 {'foo': {}}, 'charmers', 'dummy')
482
475483
476class TestUpdateCharm(MongoTestBase):484class TestUpdateCharm(MongoTestBase):
477485

Subscribers

People subscribed via source and target branches