Launchpad itself

Merge lp:~thumper/launchpad/merge-directive-handling-no-request-mirror into lp:launchpad

  1. merge-directive-handling-no-request-mirror
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11242
Proposed branch: lp:~thumper/launchpad/merge-directive-handling-no-request-mirror
Merge into: lp:launchpad
Prerequisite: lp:~thumper/launchpad/bzr-format-matching-refactoring
Diff against target: 112 lines (+14/-9)
4 files modified
database/schema/security.cfg (+1/-1)
lib/lp/code/mail/codehandler.py (+6/-1)
lib/lp/code/mail/tests/test_codehandler.py (+4/-5)
lib/lp/testing/factory.py (+3/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/merge-directive-handling-no-request-mirror
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+31019@code.launchpad.net

Commit message

Fix merge directive processing to not call requestMirror.

Description of the change

There are a few related changes here.

I filed a bug about how we shouldn't create empty branches from a merge directive if the branch is unstackable, and linked that in a couple of places in the code with an XXX comment.

The processing of the merge directive now uses branchChanged rather than requestMirror.
 - this change also needs the addition of BranchJob insert permissions for the creation of scan jobs.

The factory was told to be quiet around makeMergeDirective as it doesn't need to be proxied.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

All looks fine to me. Sorry for not catching this in the no-hosted-area branch!

review: Approve

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-07-21 09:18:56 +0000
3+++ database/schema/security.cfg 2010-07-27 05:21:00 +0000
4@@ -1636,7 +1636,7 @@
5 public.account = SELECT
6 public.accountpassword = SELECT
7 public.branch = SELECT, INSERT, UPDATE
8-public.branchjob = SELECT
9+public.branchjob = SELECT, INSERT
10 public.branchmergeproposal = SELECT, INSERT, UPDATE
11 public.branchmergeproposaljob = SELECT, INSERT
12 public.branchsubscription = SELECT, INSERT
13
14=== modified file 'lib/lp/code/mail/codehandler.py'
15--- lib/lp/code/mail/codehandler.py 2010-04-23 04:11:12 +0000
16+++ lib/lp/code/mail/codehandler.py 2010-07-27 05:21:00 +0000
17@@ -39,6 +39,7 @@
18 from canonical.launchpad.webapp.errorlog import globalErrorUtility
19 from canonical.launchpad.webapp.interfaces import ILaunchBag
20 from lazr.uri import URI
21+from lp.code.bzr import get_branch_formats
22 from lp.code.enums import BranchType, CodeReviewVote
23 from lp.code.errors import BranchMergeProposalExists, UserNotBranchReviewer
24 from lp.code.interfaces.branch import BranchCreationException
25@@ -516,11 +517,15 @@
26 except NotStacked:
27 # We don't currently support pulling in the revisions if
28 # the source branch exists and isn't stacked.
29+ # XXX Tim Penhey 2010-07-27 bug 610292
30+ # We should fail here and return an oops email to the user.
31 return db_source
32 self._pullRevisionsFromMergeDirectiveIntoSourceBranch(
33 md, target_url, bzr_source)
34 # Get the puller to pull the branch into the mirrored area.
35- db_source.requestMirror()
36+ formats = get_branch_formats(bzr_source)
37+ db_source.branchChanged(
38+ stacked_url, bzr_source.last_revision(), *formats)
39 return db_source
40 finally:
41 lp_server.stop_server()
42
43=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
44--- lib/lp/code/mail/tests/test_codehandler.py 2010-06-10 20:20:21 +0000
45+++ lib/lp/code/mail/tests/test_codehandler.py 2010-07-27 05:21:00 +0000
46@@ -11,9 +11,6 @@
47 import unittest
48
49 from bzrlib.branch import Branch
50-from bzrlib.bzrdir import BzrDir
51-from bzrlib import errors as bzr_errors
52-from bzrlib.transport import get_transport
53 from bzrlib.urlutils import join as urljoin
54 from bzrlib.workingtree import WorkingTree
55 from storm.store import Store
56@@ -893,7 +890,6 @@
57 target_tree.branch.set_public_branch(db_target_branch.bzr_identity)
58 target_tree.commit('rev1')
59 # Make sure that the created branch has been mirrored.
60- db_target_branch.startMirroring()
61 removeSecurityProxy(db_target_branch).branchChanged(
62 '', 'rev1', None, None, None)
63 source_tree = target_tree.bzrdir.sprout('source').open_workingtree()
64@@ -931,6 +927,9 @@
65 # branch that is created is an empty hosted branch. The new branch
66 # will not have a mirror requested as there are no revisions, and
67 # there is no branch created in the hosted area.
68+
69+ # XXX Tim Penhey 2010-07-27 bug 610292
70+ # We should fail here and return an oops email to the user.
71 self.useBzrBranches()
72 branch, source, message = self._createTargetSourceAndBundle(
73 format="pack-0.92")
74@@ -964,7 +963,7 @@
75 bmp = self._processMergeDirective(message)
76 source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)
77 self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type)
78- self.assertIsNot(None, bmp.source_branch.next_mirror_time)
79+ self.assertTrue(bmp.source_branch.pending_writes)
80 self.assertEqual(
81 source.last_revision(), source_bzr_branch.last_revision())
82
83
84=== modified file 'lib/lp/testing/factory.py'
85--- lib/lp/testing/factory.py 2010-07-27 05:20:59 +0000
86+++ lib/lp/testing/factory.py 2010-07-27 05:21:00 +0000
87@@ -44,6 +44,8 @@
88 from zope.security.proxy import (
89 builtin_isinstance, Proxy, ProxyFactory, removeSecurityProxy)
90
91+from bzrlib.merge_directive import MergeDirective2
92+
93 from canonical.autodecorate import AutoDecorate
94 from canonical.config import config
95 from canonical.database.constants import UTC_NOW
96@@ -2615,7 +2617,6 @@
97 context also contains the password and gpg signing mode.
98 :param sender: The `Person` that is sending the email.
99 """
100- from bzrlib.merge_directive import MergeDirective2
101 md = MergeDirective2.from_objects(
102 source_branch.repository, source_branch.last_revision(),
103 public_branch=source_branch.get_public_branch(),
104@@ -2746,7 +2747,7 @@
105 # security wrappers for them, as well as for objects created by
106 # other Python libraries.
107 unwrapped_types = (
108- DSCFile, InstanceType, Message, datetime, int, str, unicode)
109+ DSCFile, InstanceType, MergeDirective2, Message, datetime, int, str, unicode)
110
111 def is_security_proxied_or_harmless(obj):
112 """Check that the given object is security wrapped or a harmless object."""