launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01634
[Merge] lp:~rockstar/launchpad/merge-queues-api into lp:launchpad
Paul Hummer has proposed merging lp:~rockstar/launchpad/merge-queues-api into lp:launchpad with lp:~rockstar/launchpad/merge-queues-model as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This branch exposes the new branch merge queue models through the API. Since there's no UI yet, the URL resolving code also needed to be added. While doing this, I also added some more stuff to the factory method for making merge queues.
There's a lot of lint, but most of it isn't something I introduced. I could fix the lint, but it would make the diff hard to read. If you'd like me to, I'll fix the lint, but I think you'd rather me do that after the review. :)
--
https://code.launchpad.net/~rockstar/launchpad/merge-queues-api/+merge/38946
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/merge-queues-api into lp:launchpad.
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2010-10-06 18:53:53 +0000
+++ lib/lp/code/browser/configure.zcml 2010-10-20 15:36:56 +0000
@@ -1326,4 +1326,10 @@
permission="zope.Public"/>
</facet>
+ <browser:url
+ for="lp.code.interfaces.branchmergequeue.IBranchMergeQueue"
+ attribute_to_parent="owner"
+ path_expression="string:+merge-queues/${name}"
+ rootsite="code" />
+
</configure>
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2010-10-18 21:18:03 +0000
+++ lib/lp/code/configure.zcml 2010-10-20 15:36:56 +0000
@@ -22,6 +22,11 @@
provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
name="code" />
+ <!-- Branch Merge Queues -->
+ <class class="lp.code.model.branchmergequeue.BranchMergeQueue">
+ <allow interface="lp.code.interfaces.branchmergequeue.IBranchMergeQueue" />
+ </class>
+
<class class="lp.code.model.codereviewvote.CodeReviewVoteReference">
<allow interface="lp.code.interfaces.codereviewvote.ICodeReviewVoteReferencePublic"/>
<require
@@ -433,18 +438,18 @@
lp.code.interfaces.branch.IBranchAnyone
lp.code.interfaces.branch.IBranchEditableAttributes
lp.code.interfaces.branch.IBranchPublic
- lp.code.interfaces.branch.IBranchView
- "/>
+ lp.code.interfaces.branch.IBranchView"
+ attributes="merge_queue merge_queue_config"/>
<require
permission="launchpad.Edit"
interface="lp.code.interfaces.branch.IBranchEdit"
set_schema="lp.code.interfaces.branch.IBranchEditableAttributes"
- attributes="setPrivate"
+ attributes="setPrivate addToQueue setMergeQueueConfig"
set_attributes="branch_format control_format repository_format
branch_type
last_scanned last_scanned_id
last_mirrored last_mirrored_id next_mirror_time
- revision_count merge_queue mirror_failures
+ revision_count mirror_failures
stacked_on mirror_status_message"/>
<require
permission="launchpad.AnyPerson"
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2010-10-20 15:36:54 +0000
+++ lib/lp/code/interfaces/branch.py 2010-10-20 15:36:56 +0000
@@ -990,18 +990,6 @@
required=False, readonly=True,
vocabulary=ControlFormat))
- merge_queue = Reference(
- title=_('Branch Merge Queue'),
- schema=IBranchMergeQueue, required=False, readonly=True,
- description=_(
- "The branch merge queue that manages merges for this branch."))
-
- merge_queue_config = TextLine(
- title=_('Name'), required=True, constraint=branch_name_validator,
- description=_(
- "A JSON string of configuration values to send to a branch"
- "merge robot."))
-
class IBranchEdit(Interface):
"""IBranch attributes that require launchpad.Edit permission."""
@@ -1031,9 +1019,9 @@
is set, the branch gets moved into the junk namespace of the branch
owner.
- :raise: `BranchTargetError` if both project and source_package are set,
- or if either the project or source_package fail to be adapted to an
- IBranchTarget.
+ :raise: `BranchTargetError` if both project and source_package are
+ set, or if either the project or source_package fail to be
+ adapted to an IBranchTarget.
"""
def requestUpgrade():
@@ -1075,6 +1063,30 @@
:raise: CannotDeleteBranch if the branch cannot be deleted.
"""
+
+class IMergeQueueable(Interface):
+ """An interface for branches that can be queued."""
+
+ merge_queue = exported(
+ Reference(
+ title=_('Branch Merge Queue'),
+ schema=IBranchMergeQueue, required=False, readonly=True,
+ description=_(
+ "The branch merge queue that manages merges for this "
+ "branch.")))
+
+ merge_queue_config = exported(
+ TextLine(
+ title=_('Name'), required=True, readonly=True,
+ description=_(
+ "A JSON string of configuration values to send to a "
+ "branch merge robot.")))
+
+ @mutator_for(merge_queue)
+ @operation_parameters(
+ queue=Reference(title=_('Branch Merge Queue'),
+ schema=IBranchMergeQueue))
+ @export_write_operation()
def addToQueue(queue):
"""Add this branch to a specified queue.
@@ -1083,6 +1095,10 @@
:param queue: The branch merge queue that will manage the branch.
"""
+ @mutator_for(merge_queue_config)
+ @operation_parameters(
+ config=TextLine(title=_("A JSON string of config values.")))
+ @export_write_operation()
def setMergeQueueConfig(config):
"""Set the merge_queue_config property.
@@ -1094,7 +1110,7 @@
class IBranch(IBranchPublic, IBranchView, IBranchEdit,
- IBranchEditableAttributes, IBranchAnyone):
+ IBranchEditableAttributes, IBranchAnyone, IMergeQueueable):
"""A Bazaar branch."""
# Mark branches as exported entries for the Launchpad API.
=== modified file 'lib/lp/code/interfaces/branchmergequeue.py'
--- lib/lp/code/interfaces/branchmergequeue.py 2010-10-20 15:36:54 +0000
+++ lib/lp/code/interfaces/branchmergequeue.py 2010-10-20 15:36:56 +0000
@@ -10,6 +10,13 @@
'IBranchMergeQueueSource',
]
+from lazr.restful.declarations import (
+ export_as_webservice_entry,
+ export_write_operation,
+ exported,
+ mutator_for,
+ operation_parameters,
+ )
from lazr.restful.fields import (
CollectionField,
Reference,
@@ -32,47 +39,61 @@
class IBranchMergeQueue(Interface):
"""An interface for managing branch merges."""
+ export_as_webservice_entry()
+
id = Int(title=_('ID'), readonly=True, required=True)
- registrant = PublicPersonChoice(
- title=_("The user that registered the branch."),
- required=True, readonly=True,
- vocabulary='ValidPersonOrTeam')
-
- owner = PersonChoice(
- title=_('Owner'),
- required=True, readonly=True,
- vocabulary='UserTeamsParticipationPlusSelf',
- description=_("The owner of the merge queue."))
-
- name = TextLine(
- title=_('Name'), required=True,
- description=_(
- "Keep very short, unique, and descriptive, because it will "
- "be used in URLs. "
- "Examples: main, devel, release-1.0, gnome-vfs."))
-
- description = Text(
- title=_('Description'), required=False,
- description=_(
- 'A short description of the purpose of this merge queue.'))
-
- configuration = TextLine(
- title=_('Configuration'), required=False,
- description=_(
- "A JSON string of configuration values."))
-
- date_created = Datetime(
- title=_('Date Created'),
- required=True,
- readonly=True)
-
- branches = CollectionField(
- title=_('Dependent Branches'),
- description=_('A collection of branches that this queue manages.'),
- readonly=True,
- value_type=Reference(Interface))
-
+ registrant = exported(
+ PublicPersonChoice(
+ title=_("The user that registered the branch."),
+ required=True, readonly=True,
+ vocabulary='ValidPersonOrTeam'))
+
+ owner = exported(
+ PersonChoice(
+ title=_('Owner'),
+ required=True, readonly=True,
+ vocabulary='UserTeamsParticipationPlusSelf',
+ description=_("The owner of the merge queue.")))
+
+ name = exported(
+ TextLine(
+ title=_('Name'), required=True,
+ description=_(
+ "Keep very short, unique, and descriptive, because it will "
+ "be used in URLs. "
+ "Examples: main, devel, release-1.0, gnome-vfs.")))
+
+ description = exported(
+ Text(
+ title=_('Description'), required=False,
+ description=_(
+ 'A short description of the purpose of this merge queue.')))
+
+ configuration = exported(
+ TextLine(
+ title=_('Configuration'), required=False, readonly=True,
+ description=_(
+ "A JSON string of configuration values.")))
+
+ date_created = exported(
+ Datetime(
+ title=_('Date Created'),
+ required=True,
+ readonly=True))
+
+ branches = exported(
+ CollectionField(
+ title=_('Dependent Branches'),
+ description=_(
+ 'A collection of branches that this queue manages.'),
+ readonly=True,
+ value_type=Reference(Interface)))
+
+ @mutator_for(configuration)
+ @operation_parameters(
+ config=TextLine(title=_("A JSON string of configuration values.")))
+ @export_write_operation()
def setMergeQueueConfig(config):
"""Set the JSON string configuration of the merge queue.
=== modified file 'lib/lp/code/interfaces/webservice.py'
--- lib/lp/code/interfaces/webservice.py 2010-08-20 20:31:18 +0000
+++ lib/lp/code/interfaces/webservice.py 2010-10-20 15:36:56 +0000
@@ -20,6 +20,7 @@
IBranchSet,
)
from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
+from lp.code.interfaces.branchmergequeue import IBranchMergeQueue
from lp.code.interfaces.branchsubscription import IBranchSubscription
from lp.code.interfaces.codeimport import ICodeImport
from lp.code.interfaces.codereviewcomment import ICodeReviewComment
@@ -35,3 +36,4 @@
)
+IBranchMergeQueue['branches'].value_type.schema = IBranch
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2010-10-20 15:36:54 +0000
+++ lib/lp/code/model/tests/test_branch.py 2010-10-20 15:36:56 +0000
@@ -33,6 +33,7 @@
from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
from canonical.testing.layers import (
+ AppServerLayer,
DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
)
@@ -118,6 +119,7 @@
from lp.testing import (
ANONYMOUS,
celebrity_logged_in,
+ launchpadlib_for,
login,
login_person,
logout,
@@ -126,6 +128,7 @@
TestCase,
TestCaseWithFactory,
time_counter,
+ ws_object,
)
from lp.testing.factory import LaunchpadObjectFactory
from lp.translations.model.translationtemplatesbuildjob import (
@@ -2744,5 +2747,45 @@
config)
+class TestWebservice(TestCaseWithFactory):
+ """Tests for the webservice."""
+
+ layer = AppServerLayer
+
+ def test_set_merge_queue(self):
+ """Test that the merge queue can be set properly."""
+ with person_logged_in(ANONYMOUS):
+ db_queue = self.factory.makeBranchMergeQueue()
+ db_branch = self.factory.makeBranch()
+ launchpad = launchpadlib_for('test', db_branch.owner,
+ service_root="http://api.launchpad.dev:8085";)
+
+ configuration = simplejson.dumps({'test': 'make check'})
+
+ branch = ws_object(launchpad, db_branch)
+ queue = ws_object(launchpad, db_queue)
+ branch.merge_queue = queue
+ branch.lp_save()
+
+ branch2 = ws_object(launchpad, db_branch)
+ self.assertEqual(branch2.merge_queue, queue)
+
+ def test_set_configuration(self):
+ """Test the mutator for setting configuration."""
+ with person_logged_in(ANONYMOUS):
+ db_branch = self.factory.makeBranch()
+ launchpad = launchpadlib_for('test', db_branch.owner,
+ service_root="http://api.launchpad.dev:8085";)
+
+ configuration = simplejson.dumps({'test': 'make check'})
+
+ branch = ws_object(launchpad, db_branch)
+ branch.merge_queue_config = configuration
+ branch.lp_save()
+
+ branch2 = ws_object(launchpad, db_branch)
+ self.assertEqual(branch2.merge_queue_config, configuration)
+
+
def test_suite():
return TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/code/model/tests/test_branchmergequeue.py'
--- lib/lp/code/model/tests/test_branchmergequeue.py 2010-10-20 15:36:54 +0000
+++ lib/lp/code/model/tests/test_branchmergequeue.py 2010-10-20 15:36:56 +0000
@@ -5,15 +5,23 @@
import simplejson
+import transaction
+
from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.launchpad.webapp.testing import verifyObject
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.testing.layers import (
+ AppServerLayer,
+ DatabaseFunctionalLayer,
+ )
from lp.code.errors import InvalidMergeQueueConfig
from lp.code.interfaces.branchmergequeue import IBranchMergeQueue
from lp.code.model.branchmergequeue import BranchMergeQueue
from lp.testing import (
+ ANONYMOUS,
person_logged_in,
+ launchpadlib_for,
TestCaseWithFactory,
+ ws_object,
)
@@ -87,3 +95,59 @@
InvalidMergeQueueConfig,
queue.setMergeQueueConfig,
'abc')
+
+
+class TestWebservice(TestCaseWithFactory):
+
+ layer = AppServerLayer
+
+ def test_properties(self):
+ """Test that the correct properties are exposed."""
+ with person_logged_in(ANONYMOUS):
+ name = u'teh-queue'
+ description = u'Oh hai! I are a queues'
+ configuration = unicode(simplejson.dumps({'test': 'make check'}))
+
+ queuer = self.factory.makePerson()
+ db_queue = self.factory.makeBranchMergeQueue(
+ registrant=queuer, owner=queuer, name=name,
+ description=description,
+ configuration=configuration)
+ branch1 = self.factory.makeBranch()
+ with person_logged_in(branch1.owner):
+ branch1.addToQueue(db_queue)
+ branch2 = self.factory.makeBranch()
+ with person_logged_in(branch2.owner):
+ branch2.addToQueue(db_queue)
+ launchpad = launchpadlib_for('test', db_queue.owner,
+ service_root="http://api.launchpad.dev:8085";)
+ transaction.commit()
+
+ queuer = ws_object(launchpad, queuer)
+ queue = ws_object(launchpad, db_queue)
+ branch1 = ws_object(launchpad, branch1)
+ branch2 = ws_object(launchpad, branch2)
+
+ self.assertEqual(queue.registrant, queuer)
+ self.assertEqual(queue.owner, queuer)
+ self.assertEqual(queue.name, name)
+ self.assertEqual(queue.description, description)
+ self.assertEqual(queue.configuration, configuration)
+ self.assertEqual(queue.date_created, db_queue.date_created)
+ self.assertEqual(len(queue.branches), 2)
+
+ def test_set_configuration(self):
+ """Test the mutator for setting configuration."""
+ with person_logged_in(ANONYMOUS):
+ db_queue = self.factory.makeBranchMergeQueue()
+ launchpad = launchpadlib_for('test', db_queue.owner,
+ service_root="http://api.launchpad.dev:8085";)
+
+ configuration = simplejson.dumps({'test': 'make check'})
+
+ queue = ws_object(launchpad, db_queue)
+ queue.configuration = configuration
+ queue.lp_save()
+
+ queue2 = ws_object(launchpad, db_queue)
+ self.assertEqual(queue2.configuration, configuration)
=== modified file 'lib/lp/code/stories/webservice/xx-branch.txt'
--- lib/lp/code/stories/webservice/xx-branch.txt 2010-04-09 01:30:22 +0000
+++ lib/lp/code/stories/webservice/xx-branch.txt 2010-10-20 15:36:56 +0000
@@ -123,6 +123,8 @@
last_scanned_id: None
lifecycle_status: u'Development'
linked_bugs_collection_link: u'http://.../~eric/fooix/trunk/linked_bugs'
+ merge_queue_config: None
+ merge_queue_link: None
mirror_status_message: None
name: u'trunk'
owner_link: u'.../~eric'
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-10-14 20:20:47 +0000
+++ lib/lp/registry/browser/person.py 2010-10-20 15:36:56 +0000
@@ -543,6 +543,11 @@
"""Traverse to this person's recipes."""
return self.context.getRecipe(name)
+ @stepthrough('+merge-queues')
+ def traverse_merge_queue(self, name):
+ """Traverse to this person's merge queues."""
+ return self.context.getMergeQueue(name)
+
class TeamNavigation(PersonNavigation):
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2010-10-07 14:03:32 +0000
+++ lib/lp/registry/interfaces/person.py 2010-10-20 15:36:56 +0000
@@ -896,6 +896,9 @@
def getRecipe(name):
"""Return the person's recipe with the given name."""
+ def getMergeQueue(name):
+ """Return the person's merge queue with the given name."""
+
@call_with(requester=REQUEST_USER)
@export_read_operation()
def getArchiveSubscriptionURLs(requester):
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2010-10-18 21:18:03 +0000
+++ lib/lp/registry/model/person.py 2010-10-20 15:36:56 +0000
@@ -2661,6 +2661,13 @@
SourcePackageRecipe, SourcePackageRecipe.owner == self,
SourcePackageRecipe.name == name).one()
+ def getMergeQueue(self, name):
+ from lp.code.model.branchmergequeue import BranchMergeQueue
+ return Store.of(self).find(
+ BranchMergeQueue,
+ BranchMergeQueue.owner == self,
+ BranchMergeQueue.name == unicode(name)).one()
+
def isUploader(self, distribution):
"""See `IPerson`."""
permissions = getUtility(IArchivePermissionSet).componentsForUploader(
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-10-20 15:36:54 +0000
+++ lib/lp/testing/factory.py 2010-10-20 15:36:56 +0000
@@ -1098,16 +1098,23 @@
namespace = target.getNamespace(owner)
return namespace.createBranch(branch_type, name, creator)
- def makeBranchMergeQueue(self):
+ def makeBranchMergeQueue(self, registrant=None, owner=None, name=None,
+ description=None, configuration=None):
"""Create a BranchMergeQueue."""
- name = unicode(self.getUniqueString('queue'))
- owner = self.makePerson()
- description = unicode(self.getUniqueString('queue-description'))
- configuration = unicode(simplejson.dumps({
- self.getUniqueString('key'): self.getUniqueString('value')}))
+ if name is None:
+ name = unicode(self.getUniqueString('queue'))
+ if owner is None:
+ owner = self.makePerson()
+ if registrant is None:
+ registrant = self.makePerson()
+ if description is None:
+ description = unicode(self.getUniqueString('queue-description'))
+ if configuration is None:
+ configuration = unicode(simplejson.dumps({
+ self.getUniqueString('key'): self.getUniqueString('value')}))
queue = BranchMergeQueue.new(
- name, owner, owner, description, configuration)
+ name, registrant, owner, description, configuration)
return queue
def enableDefaultStackingForProduct(self, product, branch=None):
