Merge lp:~rockstar/launchpad/upgrade-branch-rc into lp:launchpad/db-devel

Proposed by Paul Hummer
Status: Merged
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/upgrade-branch-rc
Merge into: lp:launchpad/db-devel
Diff against target: 102 lines (+26/-1)
5 files modified
database/schema/security.cfg (+1/-1)
lib/lp/code/model/branch.py (+5/-0)
lib/lp/code/model/branchjob.py (+2/-0)
lib/lp/code/model/tests/test_branch.py (+15/-0)
lib/lp/code/model/tests/test_branchjob.py (+3/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/upgrade-branch-rc
Reviewer Review Type Date Requested Status
Gary Poster (community) rc Approve
Tim Penhey (community) Approve
Review via email: mp+18049@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

Hi-

  This branch fixes two release-critical issues with the branch upgrade job
stuff. The first issue is that IBranch.upgrade_pending didn't check to see if
the upgrade had failed or was finished. In those cases, it should return
false. The other issue is that the BranchUpgradeJob wasn't triggering a mirror
after it got upgraded, so the branch still thought it needed upgrading.

  The patch is the tests for those fixes and then the fixes.

Cheers,
Paul

Revision history for this message
Tim Penhey (thumper) wrote :

Looks OK to me.

review: Approve
Revision history for this message
Gary Poster (gary) :
review: Approve (rc)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2010-01-22 22:25:30 +0000
3+++ database/schema/security.cfg 2010-01-26 02:27:13 +0000
4@@ -1716,7 +1716,7 @@
5 [upgrade-branches]
6 type=user
7 groups=script
8-public.branch = SELECT
9+public.branch = SELECT, UPDATE
10 public.branchjob = SELECT
11 public.job = SELECT, UPDATE
12
13
14=== modified file 'lib/lp/code/model/branch.py'
15--- lib/lp/code/model/branch.py 2010-01-22 06:03:19 +0000
16+++ lib/lp/code/model/branch.py 2010-01-26 02:27:13 +0000
17@@ -74,6 +74,8 @@
18 IFindOfficialBranchLinks)
19 from lp.registry.interfaces.person import (
20 validate_person_not_private_membership, validate_public_person)
21+from lp.services.job.interfaces.job import JobStatus
22+from lp.services.job.model.job import Job
23 from lp.services.mail.notificationrecipientset import (
24 NotificationRecipientSet)
25
26@@ -1012,6 +1014,9 @@
27 jobs = store.find(
28 BranchJob,
29 BranchJob.branch == self,
30+ Job.id == BranchJob.jobID,
31+ Job._status != JobStatus.COMPLETED,
32+ Job._status != JobStatus.FAILED,
33 BranchJob.job_type == BranchJobType.UPGRADE_BRANCH)
34 return jobs.count() > 0
35
36
37=== modified file 'lib/lp/code/model/branchjob.py'
38--- lib/lp/code/model/branchjob.py 2010-01-26 01:20:54 +0000
39+++ lib/lp/code/model/branchjob.py 2010-01-26 02:27:13 +0000
40@@ -343,6 +343,8 @@
41 upgrade_transport.delete_tree('backup.bzr')
42 source_branch_transport.rename('.bzr', 'backup.bzr')
43 upgrade_transport.copy_tree_to_transport(source_branch_transport)
44+
45+ self.branch.requestMirror()
46 finally:
47 shutil.rmtree(upgrade_branch_path)
48
49
50=== modified file 'lib/lp/code/model/tests/test_branch.py'
51--- lib/lp/code/model/tests/test_branch.py 2010-01-22 06:03:19 +0000
52+++ lib/lp/code/model/tests/test_branch.py 2010-01-26 02:27:13 +0000
53@@ -65,6 +65,7 @@
54 from lp.registry.model.product import ProductSet
55 from lp.registry.model.sourcepackage import SourcePackage
56 from lp.registry.interfaces.pocket import PackagePublishingPocket
57+from lp.services.job.interfaces.job import JobStatus
58 from lp.testing import (
59 run_with_login, TestCase, TestCaseWithFactory, time_counter)
60 from lp.testing.factory import LaunchpadObjectFactory
61@@ -443,6 +444,20 @@
62
63 self.assertFalse(branch.upgrade_pending)
64
65+ def test_upgradePending_old_job_exists(self):
66+ # If the branch had an upgrade pending, but then the job was completed,
67+ # then upgrade_pending should return False.
68+ branch = self.factory.makeAnyBranch(
69+ branch_format=BranchFormat.BZR_BRANCH_6)
70+ owner = removeSecurityProxy(branch).owner
71+ login_person(owner)
72+ self.addCleanup(logout)
73+ branch_job = removeSecurityProxy(branch.requestUpgrade())
74+ branch_job.job.start()
75+ branch_job.job.complete()
76+
77+ self.assertFalse(branch.upgrade_pending)
78+
79
80 class TestBzrIdentity(TestCaseWithFactory):
81 """Test IBranch.bzr_identity."""
82
83=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
84--- lib/lp/code/model/tests/test_branchjob.py 2010-01-22 06:03:19 +0000
85+++ lib/lp/code/model/tests/test_branchjob.py 2010-01-26 02:27:13 +0000
86@@ -8,6 +8,7 @@
87 import datetime
88 import os
89 import shutil
90+from storm.locals import Store
91 import tempfile
92 from unittest import TestLoader
93
94@@ -263,6 +264,8 @@
95 new_branch.repository._format.get_format_string(),
96 'Bazaar repository format 2a (needs bzr 1.16 or later)\n')
97
98+ self.assertTrue(db_branch.pending_writes)
99+
100 def test_needs_no_upgrading(self):
101 # Branch upgrade job creation should raise an AssertionError if the
102 # branch does not need to be upgraded.

Subscribers

People subscribed via source and target branches

to status/vote changes: