Launchpad itself

Merge lp:~james-w/launchpad/code-import-request into lp:launchpad

  1. code-import-request
  2. Merge into devel
Proposed by James Westby
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/code-import-request
Merge into: lp:launchpad
Prerequisite: lp:~james-w/launchpad/register-code-import
Diff against target: 325 lines (+183/-17)
8 files modified
lib/lp/code/browser/branch.py (+13/-14)
lib/lp/code/configure.zcml (+2/-1)
lib/lp/code/errors.py (+23/-0)
lib/lp/code/interfaces/codeimport.py (+29/-1)
lib/lp/code/interfaces/webservice.py (+3/-1)
lib/lp/code/model/codeimport.py (+24/-0)
lib/lp/code/model/tests/test_codeimport.py (+67/-0)
lib/lp/code/stories/webservice/xx-code-import.txt (+22/-0)
To merge this branch: bzr merge lp:~james-w/launchpad/code-import-request
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+22727@code.launchpad.net

Commit message

ICodeImport.requestImport() is now available over the API.

Description of the change

Hi,

This branch adds ICodeImport.requestImport() over the API.

It first adds the method on the interface and model, with associated tests, then exports it over the
webservice with a test that calls the method using that. Lastly it ports the browser code to use
the method to reduce duplication.

The most interesting thing here is the exception handling.

To make a nice API for scripting it doesn't error if it is already queued, as the outcome is the same,
but the browser code needs to know this, so it passes a parameter to find out.

Catching the exceptions in the browser code isn't too pretty, but seemed like the best way to balance
the needs of the two APIs.

The exceptions that can be thrown during webservice access are both marked to trigger a 400 response.
This isn't ideal, as scripts will have a tough time distinguishing them, but choosing arbitrary status codes
to do that is equally ugly. The docstring at least explains what can happen and tells you how to check
for it.

Thanks,

James

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

Hi,

This adds requestImport() to ICodeImport to allow you to request
that an import happens ASAP.

Thanks,

James

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Subject to a few test cleanups mentioned on skype, this is fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2010-03-25 15:28:49 +0000
+++ lib/lp/code/browser/branch.py 2010-04-14 11:34:34 +0000
@@ -81,10 +81,12 @@
81from lp.code.browser.branchmergeproposal import (81from lp.code.browser.branchmergeproposal import (
82 latest_proposals_for_each_branch)82 latest_proposals_for_each_branch)
83from lp.code.enums import (83from lp.code.enums import (
84 BranchLifecycleStatus, BranchType, CodeImportJobState,84 BranchLifecycleStatus, BranchType,
85 CodeImportResultStatus, CodeImportReviewStatus, RevisionControlSystems,85 CodeImportResultStatus, CodeImportReviewStatus, RevisionControlSystems,
86 UICreatableBranchType)86 UICreatableBranchType)
87from lp.code.errors import InvalidBranchMergeProposal87from lp.code.errors import (
88 CodeImportAlreadyRequested, CodeImportAlreadyRunning,
89 CodeImportNotInReviewedState, InvalidBranchMergeProposal)
88from lp.code.interfaces.branch import (90from lp.code.interfaces.branch import (
89 BranchCreationForbidden, BranchExists, IBranch,91 BranchCreationForbidden, BranchExists, IBranch,
90 user_has_special_branch_access)92 user_has_special_branch_access)
@@ -1317,26 +1319,23 @@
13171319
1318 @action('Import Now', name='request')1320 @action('Import Now', name='request')
1319 def request_import_action(self, action, data):1321 def request_import_action(self, action, data):
1320 if self.context.code_import.import_job is None:1322 try:
1323 self.context.code_import.requestImport(
1324 self.user, error_if_already_requested=True)
1325 self.request.response.addNotification(
1326 "Import will run as soon as possible.")
1327 except CodeImportNotInReviewedState:
1321 self.request.response.addNotification(1328 self.request.response.addNotification(
1322 "This import is no longer being updated automatically.")1329 "This import is no longer being updated automatically.")
1323 elif (self.context.code_import.import_job.state !=1330 except CodeImportAlreadyRunning:
1324 CodeImportJobState.PENDING):
1325 assert (self.context.code_import.import_job.state ==
1326 CodeImportJobState.RUNNING)
1327 self.request.response.addNotification(1331 self.request.response.addNotification(
1328 "The import is already running.")1332 "The import is already running.")
1329 elif self.context.code_import.import_job.requesting_user is not None:1333 except CodeImportAlreadyRequested, e:
1330 user = self.context.code_import.import_job.requesting_user1334 user = e.requesting_user
1331 adapter = queryAdapter(user, IPathAdapter, 'fmt')1335 adapter = queryAdapter(user, IPathAdapter, 'fmt')
1332 self.request.response.addNotification(1336 self.request.response.addNotification(
1333 structured("The import has already been requested by %s." %1337 structured("The import has already been requested by %s." %
1334 adapter.link(None)))1338 adapter.link(None)))
1335 else:
1336 getUtility(ICodeImportJobWorkflow).requestJob(
1337 self.context.code_import.import_job, self.user)
1338 self.request.response.addNotification(
1339 "Import will run as soon as possible.")
13401339
1341 @property1340 @property
1342 def prefix(self):1341 def prefix(self):
13431342
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2010-04-12 14:51:11 +0000
+++ lib/lp/code/configure.zcml 2010-04-14 11:34:34 +0000
@@ -789,7 +789,8 @@
789 getImportDetailsForDisplay"/>789 getImportDetailsForDisplay"/>
790 <require790 <require
791 permission="launchpad.AnyPerson"791 permission="launchpad.AnyPerson"
792 attributes="tryFailingImportAgain"/>792 attributes="tryFailingImportAgain
793 requestImport"/>
793 <require794 <require
794 permission="launchpad.Edit"795 permission="launchpad.Edit"
795 attributes="updateFromData"/>796 attributes="updateFromData"/>
796797
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2010-04-05 21:38:40 +0000
+++ lib/lp/code/errors.py 2010-04-14 11:34:34 +0000
@@ -8,6 +8,9 @@
8 'BadBranchMergeProposalSearchContext',8 'BadBranchMergeProposalSearchContext',
9 'BadStateTransition',9 'BadStateTransition',
10 'BranchMergeProposalExists',10 'BranchMergeProposalExists',
11 'CodeImportAlreadyRequested',
12 'CodeImportAlreadyRunning',
13 'CodeImportNotInReviewedState',
11 'ClaimReviewFailed',14 'ClaimReviewFailed',
12 'InvalidBranchMergeProposal',15 'InvalidBranchMergeProposal',
13 'ReviewNotPending',16 'ReviewNotPending',
@@ -68,3 +71,23 @@
6871
69class UnknownBranchTypeError(Exception):72class UnknownBranchTypeError(Exception):
70 """Raised when the user specifies an unrecognized branch type."""73 """Raised when the user specifies an unrecognized branch type."""
74
75
76class CodeImportNotInReviewedState(Exception):
77 """Raised when the user requests an import of a non-automatic import."""
78
79 webservice_error(400)
80
81
82class CodeImportAlreadyRequested(Exception):
83 """Raised when the user requests an import that is already requested."""
84
85 def __init__(self, msg, requesting_user):
86 super(CodeImportAlreadyRequested, self).__init__(msg)
87 self.requesting_user = requesting_user
88
89
90class CodeImportAlreadyRunning(Exception):
91 """Raised when the user requests an import that is already running."""
92
93 webservice_error(400)
7194
=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py 2010-04-01 20:58:42 +0000
+++ lib/lp/code/interfaces/codeimport.py 2010-04-14 11:34:34 +0000
@@ -25,7 +25,8 @@
25from lp.code.interfaces.branch import IBranch25from lp.code.interfaces.branch import IBranch
2626
27from lazr.restful.declarations import (27from lazr.restful.declarations import (
28 export_as_webservice_entry, exported)28 call_with, export_as_webservice_entry, exported, export_write_operation,
29 REQUEST_USER)
29from lazr.restful.fields import ReferenceChoice30from lazr.restful.fields import ReferenceChoice
3031
3132
@@ -177,6 +178,33 @@
177 :param user: the user who is requesting the import be tried again.178 :param user: the user who is requesting the import be tried again.
178 """179 """
179180
181 @call_with(requester=REQUEST_USER)
182 @export_write_operation()
183 def requestImport(requester, error_if_already_requested=False):
184 """Request that an import be tried soon.
185
186 This method will schedule an import to happen soon for this branch.
187
188 The import must be in the Reviewed state, if not then a
189 CodeImportNotInReviewedState error will be thrown. If using the
190 API then a status code of 400 will result.
191
192 If the import is already running then a CodeImportAlreadyRunning
193 error will be thrown. If using the API then a status code of
194 400 will result.
195
196 The two cases can be distinguished over the API by seeing if the
197 exception names appear in the body of the response.
198
199 If used over the API and the request has already been made then this
200 method will silently do nothing.
201 If called internally then the error_if_already_requested parameter
202 controls whether a CodeImportAlreadyRequested exception will be
203 thrown in that situation.
204
205 :return: None
206 """
207
180208
181class ICodeImportSet(Interface):209class ICodeImportSet(Interface):
182 """Interface representing the set of code imports."""210 """Interface representing the set of code imports."""
183211
=== modified file 'lib/lp/code/interfaces/webservice.py'
--- lib/lp/code/interfaces/webservice.py 2010-04-01 23:04:10 +0000
+++ lib/lp/code/interfaces/webservice.py 2010-04-14 11:34:34 +0000
@@ -5,7 +5,9 @@
55
6# The exceptions are imported so that they can produce the special6# The exceptions are imported so that they can produce the special
7# status code defined by webservice_error when they are raised.7# status code defined by webservice_error when they are raised.
8from lp.code.errors import BranchMergeProposalExists8from lp.code.errors import (
9 BranchMergeProposalExists, CodeImportAlreadyRunning,
10 CodeImportNotInReviewedState)
9from lp.code.interfaces.branch import (11from lp.code.interfaces.branch import (
10 IBranch, IBranchSet, BranchCreatorNotMemberOfOwnerTeam,12 IBranch, IBranchSet, BranchCreatorNotMemberOfOwnerTeam,
11 BranchCreatorNotOwner, BranchExists)13 BranchCreatorNotOwner, BranchExists)
1214
=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py 2010-04-01 20:58:42 +0000
+++ lib/lp/code/model/codeimport.py 2010-04-14 11:34:34 +0000
@@ -37,6 +37,9 @@
37from lp.code.enums import (37from lp.code.enums import (
38 BranchType, CodeImportJobState, CodeImportResultStatus,38 BranchType, CodeImportJobState, CodeImportResultStatus,
39 CodeImportReviewStatus, RevisionControlSystems)39 CodeImportReviewStatus, RevisionControlSystems)
40from lp.code.errors import (
41 CodeImportAlreadyRequested, CodeImportAlreadyRunning,
42 CodeImportNotInReviewedState)
40from lp.code.interfaces.codeimport import ICodeImport, ICodeImportSet43from lp.code.interfaces.codeimport import ICodeImport, ICodeImportSet
41from lp.code.interfaces.codeimportevent import ICodeImportEventSet44from lp.code.interfaces.codeimportevent import ICodeImportEventSet
42from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow45from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
@@ -196,6 +199,27 @@
196 {'review_status': CodeImportReviewStatus.REVIEWED}, user)199 {'review_status': CodeImportReviewStatus.REVIEWED}, user)
197 getUtility(ICodeImportJobWorkflow).requestJob(self.import_job, user)200 getUtility(ICodeImportJobWorkflow).requestJob(self.import_job, user)
198201
202 def requestImport(self, requester, error_if_already_requested=False):
203 """See `ICodeImport`."""
204 if self.import_job is None: # not in automatic mode
205 raise CodeImportNotInReviewedState("This code import is %s, and "
206 "must be Reviewed for you to call requestImport."
207 % self.review_status.name)
208 if (self.import_job.state != CodeImportJobState.PENDING):
209 assert (self.import_job.state == CodeImportJobState.RUNNING)
210 # Already running
211 raise CodeImportAlreadyRunning("This code import is already "
212 "running.")
213 elif self.import_job.requesting_user is not None:
214 if error_if_already_requested:
215 raise CodeImportAlreadyRequested("This code import has "
216 "already been requested to run.",
217 self.import_job.requesting_user)
218 else:
219 getUtility(ICodeImportJobWorkflow).requestJob(
220 self.import_job, requester)
221 return None
222
199223
200class CodeImportSet:224class CodeImportSet:
201 """See `ICodeImportSet`."""225 """See `ICodeImportSet`."""
202226
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py 2010-04-01 20:58:42 +0000
+++ lib/lp/code/model/tests/test_codeimport.py 2010-04-14 11:34:34 +0000
@@ -12,6 +12,11 @@
12from zope.component import getUtility12from zope.component import getUtility
13from zope.security.proxy import removeSecurityProxy13from zope.security.proxy import removeSecurityProxy
1414
15from canonical.launchpad.testing.codeimporthelpers import (
16 make_running_import)
17from lp.code.errors import (
18 CodeImportAlreadyRequested, CodeImportAlreadyRunning,
19 CodeImportNotInReviewedState)
15from lp.code.model.codeimport import CodeImportSet20from lp.code.model.codeimport import CodeImportSet
16from lp.code.model.codeimportevent import CodeImportEvent21from lp.code.model.codeimportevent import CodeImportEvent
17from lp.code.model.codeimportjob import CodeImportJob, CodeImportJobSet22from lp.code.model.codeimportjob import CodeImportJob, CodeImportJobSet
@@ -620,5 +625,67 @@
620 requester, code_import.import_job.requesting_user)625 requester, code_import.import_job.requesting_user)
621626
622627
628class TestRequestImport(TestCaseWithFactory):
629 """Tests for `ICodeImport.requestImport`."""
630
631 layer = DatabaseFunctionalLayer
632
633 def setUp(self):
634 # We have to be logged in to request imports
635 TestCaseWithFactory.setUp(self, user='no-priv@canonical.com')
636
637 def test_requestsJob(self):
638 code_import = self.factory.makeCodeImport(
639 git_repo_url=self.factory.getUniqueURL())
640 requester = self.factory.makePerson()
641 old_date = code_import.import_job.date_due
642 code_import.requestImport(requester)
643 self.assertEqual(requester, code_import.import_job.requesting_user)
644 self.assertTrue(code_import.import_job.date_due <= old_date)
645
646 def test_noop_if_already_requested(self):
647 code_import = self.factory.makeCodeImport(
648 git_repo_url=self.factory.getUniqueURL())
649 requester = self.factory.makePerson()
650 code_import.requestImport(requester)
651 old_date = code_import.import_job.date_due
652 code_import.requestImport(requester)
653 # The checks don't matter so much, it's more that we don't get
654 # an exception.
655 self.assertEqual(requester, code_import.import_job.requesting_user)
656 self.assertEqual(old_date, code_import.import_job.date_due)
657
658 def test_optional_error_if_already_requested(self):
659 code_import = self.factory.makeCodeImport(
660 git_repo_url=self.factory.getUniqueURL())
661 requester = self.factory.makePerson()
662 code_import.requestImport(requester)
663 old_date = code_import.import_job.date_due
664 e = self.assertRaises(
665 CodeImportAlreadyRequested, code_import.requestImport, requester,
666 error_if_already_requested=True)
667 self.assertEqual(requester, e.requesting_user)
668
669 def test_exception_on_disabled(self):
670 # get an SVN request, which isn't reviewed by default
671 code_import = self.factory.makeCodeImport(
672 svn_branch_url=self.factory.getUniqueURL())
673 requester = self.factory.makePerson()
674 # which leads to an exception if we try and ask for an import
675 self.assertRaises(
676 CodeImportNotInReviewedState, code_import.requestImport,
677 requester)
678
679 def test_exception_if_already_running(self):
680 code_import = self.factory.makeCodeImport(
681 git_repo_url=self.factory.getUniqueURL())
682 code_import = make_running_import(factory=self.factory,
683 code_import=code_import)
684 requester = self.factory.makePerson()
685 self.assertRaises(
686 CodeImportAlreadyRunning, code_import.requestImport,
687 requester)
688
689
623def test_suite():690def test_suite():
624 return unittest.TestLoader().loadTestsFromName(__name__)691 return unittest.TestLoader().loadTestsFromName(__name__)
625692
=== modified file 'lib/lp/code/stories/webservice/xx-code-import.txt'
--- lib/lp/code/stories/webservice/xx-code-import.txt 2010-04-13 14:56:00 +0000
+++ lib/lp/code/stories/webservice/xx-code-import.txt 2010-04-14 11:34:34 +0000
@@ -228,3 +228,25 @@
228 None228 None
229 >>> print representation['date_last_successful']229 >>> print representation['date_last_successful']
230 None230 None
231
232
233== Requesting an Import ==
234
235You can request that an approved, working import happen soon over the
236API using the requestImport() method.
237
238 >>> login(ANONYMOUS)
239 >>> git_import = factory.makeProductCodeImport(
240 ... registrant=person, product=product, branch_name='git-import',
241 ... git_repo_url=factory.getUniqueURL())
242 >>> git_import_url = (
243 ... '/' + git_import.branch.unique_name + '/+code-import')
244 >>> logout()
245 >>> import_webservice = webservice_for_person(
246 ... person, permission=OAuthPermission.WRITE_PUBLIC)
247 >>> response = import_webservice.named_post(
248 ... git_import_url, 'requestImport')
249 >>> print response.status
250 200
251 >>> print response.jsonBody()
252 None