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
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2010-07-21 09:18:56 +0000
+++ database/schema/security.cfg 2010-07-27 05:21:00 +0000
@@ -1636,7 +1636,7 @@
1636public.account = SELECT1636public.account = SELECT
1637public.accountpassword = SELECT1637public.accountpassword = SELECT
1638public.branch = SELECT, INSERT, UPDATE1638public.branch = SELECT, INSERT, UPDATE
1639public.branchjob = SELECT1639public.branchjob = SELECT, INSERT
1640public.branchmergeproposal = SELECT, INSERT, UPDATE1640public.branchmergeproposal = SELECT, INSERT, UPDATE
1641public.branchmergeproposaljob = SELECT, INSERT1641public.branchmergeproposaljob = SELECT, INSERT
1642public.branchsubscription = SELECT, INSERT1642public.branchsubscription = SELECT, INSERT
16431643
=== modified file 'lib/lp/code/mail/codehandler.py'
--- lib/lp/code/mail/codehandler.py 2010-04-23 04:11:12 +0000
+++ lib/lp/code/mail/codehandler.py 2010-07-27 05:21:00 +0000
@@ -39,6 +39,7 @@
39from canonical.launchpad.webapp.errorlog import globalErrorUtility39from canonical.launchpad.webapp.errorlog import globalErrorUtility
40from canonical.launchpad.webapp.interfaces import ILaunchBag40from canonical.launchpad.webapp.interfaces import ILaunchBag
41from lazr.uri import URI41from lazr.uri import URI
42from lp.code.bzr import get_branch_formats
42from lp.code.enums import BranchType, CodeReviewVote43from lp.code.enums import BranchType, CodeReviewVote
43from lp.code.errors import BranchMergeProposalExists, UserNotBranchReviewer44from lp.code.errors import BranchMergeProposalExists, UserNotBranchReviewer
44from lp.code.interfaces.branch import BranchCreationException45from lp.code.interfaces.branch import BranchCreationException
@@ -516,11 +517,15 @@
516 except NotStacked:517 except NotStacked:
517 # We don't currently support pulling in the revisions if518 # We don't currently support pulling in the revisions if
518 # the source branch exists and isn't stacked.519 # the source branch exists and isn't stacked.
520 # XXX Tim Penhey 2010-07-27 bug 610292
521 # We should fail here and return an oops email to the user.
519 return db_source522 return db_source
520 self._pullRevisionsFromMergeDirectiveIntoSourceBranch(523 self._pullRevisionsFromMergeDirectiveIntoSourceBranch(
521 md, target_url, bzr_source)524 md, target_url, bzr_source)
522 # Get the puller to pull the branch into the mirrored area.525 # Get the puller to pull the branch into the mirrored area.
523 db_source.requestMirror()526 formats = get_branch_formats(bzr_source)
527 db_source.branchChanged(
528 stacked_url, bzr_source.last_revision(), *formats)
524 return db_source529 return db_source
525 finally:530 finally:
526 lp_server.stop_server()531 lp_server.stop_server()
527532
=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py 2010-06-10 20:20:21 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py 2010-07-27 05:21:00 +0000
@@ -11,9 +11,6 @@
11import unittest11import unittest
1212
13from bzrlib.branch import Branch13from bzrlib.branch import Branch
14from bzrlib.bzrdir import BzrDir
15from bzrlib import errors as bzr_errors
16from bzrlib.transport import get_transport
17from bzrlib.urlutils import join as urljoin14from bzrlib.urlutils import join as urljoin
18from bzrlib.workingtree import WorkingTree15from bzrlib.workingtree import WorkingTree
19from storm.store import Store16from storm.store import Store
@@ -893,7 +890,6 @@
893 target_tree.branch.set_public_branch(db_target_branch.bzr_identity)890 target_tree.branch.set_public_branch(db_target_branch.bzr_identity)
894 target_tree.commit('rev1')891 target_tree.commit('rev1')
895 # Make sure that the created branch has been mirrored.892 # Make sure that the created branch has been mirrored.
896 db_target_branch.startMirroring()
897 removeSecurityProxy(db_target_branch).branchChanged(893 removeSecurityProxy(db_target_branch).branchChanged(
898 '', 'rev1', None, None, None)894 '', 'rev1', None, None, None)
899 source_tree = target_tree.bzrdir.sprout('source').open_workingtree()895 source_tree = target_tree.bzrdir.sprout('source').open_workingtree()
@@ -931,6 +927,9 @@
931 # branch that is created is an empty hosted branch. The new branch927 # branch that is created is an empty hosted branch. The new branch
932 # will not have a mirror requested as there are no revisions, and928 # will not have a mirror requested as there are no revisions, and
933 # there is no branch created in the hosted area.929 # there is no branch created in the hosted area.
930
931 # XXX Tim Penhey 2010-07-27 bug 610292
932 # We should fail here and return an oops email to the user.
934 self.useBzrBranches()933 self.useBzrBranches()
935 branch, source, message = self._createTargetSourceAndBundle(934 branch, source, message = self._createTargetSourceAndBundle(
936 format="pack-0.92")935 format="pack-0.92")
@@ -964,7 +963,7 @@
964 bmp = self._processMergeDirective(message)963 bmp = self._processMergeDirective(message)
965 source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)964 source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)
966 self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type)965 self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type)
967 self.assertIsNot(None, bmp.source_branch.next_mirror_time)966 self.assertTrue(bmp.source_branch.pending_writes)
968 self.assertEqual(967 self.assertEqual(
969 source.last_revision(), source_bzr_branch.last_revision())968 source.last_revision(), source_bzr_branch.last_revision())
970969
971970
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-07-27 05:20:59 +0000
+++ lib/lp/testing/factory.py 2010-07-27 05:21:00 +0000
@@ -44,6 +44,8 @@
44from zope.security.proxy import (44from zope.security.proxy import (
45 builtin_isinstance, Proxy, ProxyFactory, removeSecurityProxy)45 builtin_isinstance, Proxy, ProxyFactory, removeSecurityProxy)
4646
47from bzrlib.merge_directive import MergeDirective2
48
47from canonical.autodecorate import AutoDecorate49from canonical.autodecorate import AutoDecorate
48from canonical.config import config50from canonical.config import config
49from canonical.database.constants import UTC_NOW51from canonical.database.constants import UTC_NOW
@@ -2615,7 +2617,6 @@
2615 context also contains the password and gpg signing mode.2617 context also contains the password and gpg signing mode.
2616 :param sender: The `Person` that is sending the email.2618 :param sender: The `Person` that is sending the email.
2617 """2619 """
2618 from bzrlib.merge_directive import MergeDirective2
2619 md = MergeDirective2.from_objects(2620 md = MergeDirective2.from_objects(
2620 source_branch.repository, source_branch.last_revision(),2621 source_branch.repository, source_branch.last_revision(),
2621 public_branch=source_branch.get_public_branch(),2622 public_branch=source_branch.get_public_branch(),
@@ -2746,7 +2747,7 @@
2746# security wrappers for them, as well as for objects created by2747# security wrappers for them, as well as for objects created by
2747# other Python libraries.2748# other Python libraries.
2748unwrapped_types = (2749unwrapped_types = (
2749 DSCFile, InstanceType, Message, datetime, int, str, unicode)2750 DSCFile, InstanceType, MergeDirective2, Message, datetime, int, str, unicode)
27502751
2751def is_security_proxied_or_harmless(obj):2752def is_security_proxied_or_harmless(obj):
2752 """Check that the given object is security wrapped or a harmless object."""2753 """Check that the given object is security wrapped or a harmless object."""