launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04799
[Merge] lp:~jelmer/launchpad/no-code-import-approval into lp:launchpad
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/no-code-import-approval into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #418932 in Launchpad itself: "Don't require approval for VCS imports"
https://bugs.launchpad.net/launchpad/+bug/418932
For more details, see:
https://code.launchpad.net/~jelmer/launchpad/no-code-import-approval/+merge/73186
Remove the requirement for svn and cvs imports to be pre-approved by a Launchpad admin, fixing bug #418932.
--
https://code.launchpad.net/~jelmer/launchpad/no-code-import-approval/+merge/73186
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/no-code-import-approval into lp:launchpad.
=== modified file 'lib/lp/code/doc/codeimport-event.txt'
--- lib/lp/code/doc/codeimport-event.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/code/doc/codeimport-event.txt 2011-08-28 19:23:29 +0000
@@ -120,7 +120,7 @@
>>> print_items(svn_create_event)
CODE_IMPORT <muted>
OWNER ...
- REVIEW_STATUS u'NEW'
+ REVIEW_STATUS u'REVIEWED'
ASSIGNEE None
UPDATE_INTERVAL None
URL u'svn://svn.example.com/trunk'
@@ -144,7 +144,7 @@
>>> print_items(cvs_create_event)
CODE_IMPORT <muted>
OWNER ...
- REVIEW_STATUS u'NEW'
+ REVIEW_STATUS u'REVIEWED'
ASSIGNEE None
UPDATE_INTERVAL None
CVS_ROOT u':pserver:anonymous@xxxxxxxxxxxxxxx:/cvsroot'
@@ -206,7 +206,7 @@
>>> from lp.code.enums import CodeImportReviewStatus
>>> removeSecurityProxy(svn_import).review_status = (
- ... CodeImportReviewStatus.REVIEWED)
+ ... CodeImportReviewStatus.SUSPENDED)
After applying changes, the newModify method can create an event that
details the changes that have been applied.
@@ -234,8 +234,8 @@
>>> print_items(modify_event)
CODE_IMPORT <muted>
OWNER ...
- REVIEW_STATUS u'REVIEWED'
- OLD_REVIEW_STATUS u'NEW'
+ REVIEW_STATUS u'SUSPENDED'
+ OLD_REVIEW_STATUS u'REVIEWED'
ASSIGNEE None
UPDATE_INTERVAL None
URL u'svn://svn.example.com/trunk'
=== modified file 'lib/lp/code/doc/codeimport.txt'
--- lib/lp/code/doc/codeimport.txt 2011-06-28 17:13:42 +0000
+++ lib/lp/code/doc/codeimport.txt 2011-08-28 19:23:29 +0000
@@ -243,7 +243,7 @@
>>> code_import = factory.makeProductCodeImport(
... svn_branch_url='http://svn.example.com/project')
>>> print code_import.review_status.title
- Pending Review
+ Reviewed
When an import operator updates the code import emails are sent out to
the branch subscribers and members of VCS Imports that describe the
@@ -275,8 +275,6 @@
To: david.allouche@xxxxxxxxxxxxx, ...
Subject: Code import product.../name... status: Reviewed
<BLANKLINE>
- The import has been approved and an import will start shortly.
- <BLANKLINE>
... is now being imported from:
http://svn.example.com/project/trunk
instead of:
@@ -293,8 +291,6 @@
To: import@xxxxxxxxxxx
Subject: Code import product.../name... status: Reviewed
<BLANKLINE>
- The import has been approved and an import will start shortly.
- <BLANKLINE>
... is now being imported from:
http://svn.example.com/project/trunk
instead of:
=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py 2011-08-24 21:17:35 +0000
+++ lib/lp/code/model/codeimport.py 2011-08-28 19:23:29 +0000
@@ -198,7 +198,8 @@
setattr(self, name, value)
if 'review_status' in data:
if data['review_status'] == CodeImportReviewStatus.REVIEWED:
- CodeImportJobWorkflow().newJob(self)
+ if self.import_job is None:
+ CodeImportJobWorkflow().newJob(self)
else:
self._removeJob()
event = event_set.newModify(self, user, token)
@@ -264,13 +265,8 @@
"Don't know how to sanity check source details for unknown "
"rcs_type %s"%rcs_type)
if review_status is None:
- # Auto approve git and hg imports.
- if rcs_type in (
- RevisionControlSystems.GIT, RevisionControlSystems.HG,
- RevisionControlSystems.BZR):
- review_status = CodeImportReviewStatus.REVIEWED
- else:
- review_status = CodeImportReviewStatus.NEW
+ # Auto approve imports.
+ review_status = CodeImportReviewStatus.REVIEWED
if not target.supports_code_imports:
raise AssertionError("%r doesn't support code imports" % target)
if owner is None:
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2011-08-24 16:21:07 +0000
+++ lib/lp/code/model/tests/test_branch.py 2011-08-28 19:23:29 +0000
@@ -1510,7 +1510,6 @@
"""break_links allows deleting a code import branch."""
code_import = self.factory.makeCodeImport()
code_import_id = code_import.id
- self.factory.makeCodeImportJob(code_import)
code_import.branch.destroySelf(break_references=True)
self.assertRaises(
SQLObjectNotFound, CodeImport.get, code_import_id)
@@ -1575,7 +1574,6 @@
"""DeleteCodeImport.__call__ must delete the CodeImport."""
code_import = self.factory.makeCodeImport()
code_import_id = code_import.id
- self.factory.makeCodeImportJob(code_import)
DeleteCodeImport(code_import)()
self.assertRaises(
SQLObjectNotFound, CodeImport.get, code_import_id)
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py 2011-08-28 08:36:14 +0000
+++ lib/lp/code/model/tests/test_codeimport.py 2011-08-28 19:23:29 +0000
@@ -56,20 +56,6 @@
layer = DatabaseFunctionalLayer
- def test_new_svn_import(self):
- """A new subversion code import should have NEW status."""
- code_import = CodeImportSet().new(
- registrant=self.factory.makePerson(),
- target=IBranchTarget(self.factory.makeProduct()),
- branch_name='imported',
- rcs_type=RevisionControlSystems.SVN,
- url=self.factory.getUniqueURL())
- self.assertEqual(
- CodeImportReviewStatus.NEW,
- code_import.review_status)
- # No job is created for the import.
- self.assertIs(None, code_import.import_job)
-
def test_new_svn_import_svn_scheme(self):
"""A subversion import can use the svn:// scheme."""
code_import = CodeImportSet().new(
@@ -79,10 +65,10 @@
rcs_type=RevisionControlSystems.SVN,
url=self.factory.getUniqueURL(scheme="svn"))
self.assertEqual(
- CodeImportReviewStatus.NEW,
+ CodeImportReviewStatus.REVIEWED,
code_import.review_status)
# No job is created for the import.
- self.assertIs(None, code_import.import_job)
+ self.assertIsNot(None, code_import.import_job)
def test_reviewed_svn_import(self):
"""A specific review status can be set for a new import."""
@@ -92,30 +78,15 @@
branch_name='imported',
rcs_type=RevisionControlSystems.SVN,
url=self.factory.getUniqueURL(),
- review_status=CodeImportReviewStatus.REVIEWED)
+ review_status=None)
self.assertEqual(
CodeImportReviewStatus.REVIEWED,
code_import.review_status)
# A job is created for the import.
self.assertIsNot(None, code_import.import_job)
- def test_new_cvs_import(self):
- """A new CVS code import should have NEW status."""
- code_import = CodeImportSet().new(
- registrant=self.factory.makePerson(),
- target=IBranchTarget(self.factory.makeProduct()),
- branch_name='imported',
- rcs_type=RevisionControlSystems.CVS,
- cvs_root=self.factory.getUniqueURL(),
- cvs_module='module')
- self.assertEqual(
- CodeImportReviewStatus.NEW,
- code_import.review_status)
- # No job is created for the import.
- self.assertIs(None, code_import.import_job)
-
- def test_reviewed_cvs_import(self):
- """A specific review status can be set for a new import."""
+ def test_cvs_import_reviewed(self):
+ """A new CVS code import should have REVIEWED status."""
code_import = CodeImportSet().new(
registrant=self.factory.makePerson(),
target=IBranchTarget(self.factory.makeProduct()),
@@ -123,7 +94,7 @@
rcs_type=RevisionControlSystems.CVS,
cvs_root=self.factory.getUniqueURL(),
cvs_module='module',
- review_status=CodeImportReviewStatus.REVIEWED)
+ review_status=None)
self.assertEqual(
CodeImportReviewStatus.REVIEWED,
code_import.review_status)
@@ -275,8 +246,7 @@
"""Ensure deleting CodeImport objects deletes associated jobs."""
code_import = self.factory.makeCodeImport()
login_person(getUtility(ILaunchpadCelebrities).vcs_imports.teamowner)
- code_import_job = self.factory.makeCodeImportJob(code_import)
- job_id = code_import_job.id
+ job_id = code_import.import_job.id
CodeImportJobSet().getById(job_id)
job = CodeImportJobSet().getById(job_id)
assert job is not None
@@ -644,7 +614,8 @@
# tryFailingImportAgain only succeeds for imports that are FAILING.
outcomes = {}
for status in CodeImportReviewStatus.items:
- code_import = self.factory.makeCodeImport()
+ code_import = self.factory.makeCodeImport(
+ review_status=CodeImportReviewStatus.NEW)
code_import.updateFromData(
{'review_status': status}, self.factory.makePerson())
try:
@@ -726,9 +697,10 @@
self.assertEqual(requester, e.requesting_user)
def test_exception_on_disabled(self):
- # get an SVN request, which isn't reviewed by default
+ # get an SVN request which is suspended
code_import = self.factory.makeCodeImport(
- svn_branch_url=self.factory.getUniqueURL())
+ svn_branch_url=self.factory.getUniqueURL(),
+ review_status=CodeImportReviewStatus.SUSPENDED)
requester = self.factory.makePerson()
# which leads to an exception if we try and ask for an import
self.assertRaises(
=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py 2011-06-29 12:00:39 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py 2011-08-28 19:23:29 +0000
@@ -115,7 +115,8 @@
def makeJob(self, state, date_due_delta, requesting_user=None):
"""Create a CodeImportJob object from a spec."""
- code_import = self.factory.makeCodeImport()
+ code_import = self.factory.makeCodeImport(
+ review_status=CodeImportReviewStatus.NEW)
job = self.factory.makeCodeImportJob(code_import)
if state == CodeImportJobState.RUNNING:
getUtility(ICodeImportJobWorkflow).startJob(job, self.machine)
@@ -387,11 +388,12 @@
def test_wrongReviewStatus(self):
# CodeImportJobWorkflow.newJob fails if the CodeImport review_status
# is different from REVIEWED.
- new_import = self.factory.makeCodeImport()
+ new_import = self.factory.makeCodeImport(
+ review_status=CodeImportReviewStatus.SUSPENDED)
branch_name = new_import.branch.unique_name
# Testing newJob failure.
self.assertFailure(
- "Review status of %s is not REVIEWED: NEW" % (branch_name,),
+ "Review status of %s is not REVIEWED: SUSPENDED" % (branch_name,),
getUtility(ICodeImportJobWorkflow).newJob, new_import)
def test_existingJob(self):
@@ -417,14 +419,17 @@
# If there is no CodeImportResult for the CodeImport, then the new
# CodeImportJob has date_due set to UTC_NOW.
code_import = self.getCodeImportForDateDueTest()
- job = getUtility(ICodeImportJobWorkflow).newJob(code_import)
- self.assertSqlAttributeEqualsDate(job, 'date_due', UTC_NOW)
+ self.assertSqlAttributeEqualsDate(code_import.import_job, 'date_due',
+ UTC_NOW)
def test_dateDueRecentPreviousResult(self):
# If there is a CodeImportResult for the CodeImport that is more
# recent than the effective_update_interval, then the new
# CodeImportJob has date_due set in the future.
code_import = self.getCodeImportForDateDueTest()
+ # A code import job is automatically started when a reviewed code import
+ # is created. Remove it, so a "clean" one can be created later.
+ removeSecurityProxy(code_import).import_job.destroySelf()
# Create a CodeImportResult that started a long time ago. This one
# must be superseded by the more recent one created below.
machine = self.factory.makeCodeImportMachine()
@@ -469,8 +474,8 @@
date_job_started=datetime(2000, 1, 1, 12, 0, 0, tzinfo=UTC),
date_created=datetime(2000, 1, 1, 12, 5, 0, tzinfo=UTC))
# When we create the job, its date due must be set to UTC_NOW.
- job = getUtility(ICodeImportJobWorkflow).newJob(code_import)
- self.assertSqlAttributeEqualsDate(job, 'date_due', UTC_NOW)
+ self.assertSqlAttributeEqualsDate(code_import.import_job, 'date_due',
+ UTC_NOW)
class TestCodeImportJobWorkflowDeletePendingJob(TestCaseWithFactory,
@@ -500,7 +505,8 @@
def test_noJob(self):
# CodeImportJobWorkflow.deletePendingJob fails if the
# CodeImport is not associated to a CodeImportJob.
- new_import = self.factory.makeCodeImport()
+ new_import = self.factory.makeCodeImport(
+ review_status=CodeImportReviewStatus.NEW)
branch_name = new_import.branch.unique_name
# Testing deletePendingJob failure.
self.assertFailure(
@@ -578,7 +584,7 @@
# CodeImportJobWorkflow.requestJob sets requesting_user and
# date_due if the current date_due is in the future.
code_import = self.factory.makeCodeImport()
- pending_job = self.factory.makeCodeImportJob(code_import)
+ pending_job = code_import.import_job
person = self.factory.makePerson()
# Set date_due in the future. ICodeImportJob does not allow setting
# date_due, so we must use removeSecurityProxy.
@@ -877,8 +883,7 @@
unchecked_result_fields.difference_update(['log_file', 'status'])
code_import = self.factory.makeCodeImport()
- removeSecurityProxy(code_import).review_status = \
- CodeImportReviewStatus.REVIEWED
+ removeSecurityProxy(code_import).import_job.destroySelf()
self.assertFinishJobPassesThroughJobField(
'code_import', 'code_import', code_import)
unchecked_result_fields.remove('code_import')
=== modified file 'lib/lp/code/stories/codeimport/xx-admin-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2010-04-28 02:49:58 +0000
+++ lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2011-08-28 19:23:29 +0000
@@ -61,9 +61,11 @@
... print extract_text(div)
>>> import_browser.open(svn_import_location)
>>> print_import_details(import_browser)
- Import Status: Pending Review
+ Import Status: Reviewed
This branch is an import of the Subversion branch
from svn://svn.example.com/fooix/trunk.
+ The next import is scheduled to run
+ as soon as possible.
Edit import source or review import
@@ -76,7 +78,6 @@
>>> import_browser.getLink('Edit import source or review import').click()
>>> print_submit_buttons(import_browser.contents)
Update
- Approve
Mark Invalid
Suspend
Mark Failing
@@ -190,24 +191,6 @@
The code import has been updated.
-Approving an import
-+++++++++++++++++++
-
-When a code import is approved, a pending job is created for it.
-
- >>> import_browser.open(svn_import_location + '/+edit-import')
- >>> import_browser.getControl('Approve').click()
- >>> print_import_details(import_browser)
- Import Status: Reviewed
- ...
- The next import is scheduled to run as soon as possible.
- Edit import source or review import
-
- >>> for message in get_feedback_messages(import_browser.contents):
- ... print extract_text(message)
- The code import has been approved.
-
-
Invalidating an import
++++++++++++++++++++++
=== modified file 'lib/lp/code/stories/codeimport/xx-create-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-create-codeimport.txt 2010-09-28 19:25:54 +0000
+++ lib/lp/code/stories/codeimport/xx-create-codeimport.txt 2011-08-28 19:23:29 +0000
@@ -72,9 +72,11 @@
When the user clicks continue, the import branch is created
>>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
- Import Status: Pending Review
+ Import Status: Reviewed
This branch is an import of the Subversion branch
from http://svn.example.com/firefox/trunk.
+ The next import is scheduled to run
+ as soon as possible.
>>> browser.getLink("http://svn.example.com/firefox/trunk";)
<Link text='http://svn.example.com/firefox/trunk'
url='http://svn.example.com/firefox/trunk'>
@@ -98,9 +100,11 @@
>>> browser.getControl('Project').value = "firefox"
>>> browser.getControl('Request Import').click()
>>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
- Import Status: Pending Review
+ Import Status: Reviewed
This branch is an import of the Subversion branch
from http://user:password@xxxxxxxxxxxxxxx/firefox/trunk.
+ The next import is scheduled to run
+ as soon as possible.
Requesting a Git import
@@ -165,10 +169,11 @@
>>> browser.getControl('Request Import').click()
>>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
- Import Status: Pending Review
+ Import Status: Reviewed
This branch is an import of the CVS module firefox from
:pserver:anonymous@xxxxxxxxxxxxxxx:/mozilla/cvs.
-
+ The next import is scheduled to run
+ as soon as possible.
Requesting a CVS import with invalid information
================================================
=== modified file 'lib/lp/code/stories/codeimport/xx-edit-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-edit-codeimport.txt 2010-03-18 15:39:58 +0000
+++ lib/lp/code/stories/codeimport/xx-edit-codeimport.txt 2011-08-28 19:23:29 +0000
@@ -43,9 +43,11 @@
... print extract_text(div)
>>> anon_browser.open(svn_import_location)
>>> print_import_details(anon_browser)
- Import Status: Pending Review
+ Import Status: Reviewed
This branch is an import of the Subversion branch
from svn://svn.example.com/fooix/trunk.
+ The next import is scheduled to run
+ as soon as possible.
Because it's an svn:// URL, it doesn't get linkified:
=== modified file 'lib/lp/code/stories/webservice/xx-code-import.txt'
--- lib/lp/code/stories/webservice/xx-code-import.txt 2010-04-16 05:03:03 +0000
+++ lib/lp/code/stories/webservice/xx-code-import.txt 2011-08-28 19:23:29 +0000
@@ -55,7 +55,7 @@
>>> print representation['branch_link']
http://.../~import-owner/scruff/import
>>> print representation['review_status']
- Pending Review
+ Reviewed
>>> print representation['rcs_type']
Subversion via CSCVS
>>> print representation['url']
@@ -105,7 +105,7 @@
>>> print representation['branch_link']
http://.../~import-owner/scruffbuntu/manic/scruff/import
>>> print representation['review_status']
- Pending Review
+ Reviewed
>>> print representation['rcs_type']
Subversion via CSCVS
>>> print representation['url']
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-08-27 16:28:52 +0000
+++ lib/lp/testing/factory.py 2011-08-28 19:23:29 +0000
@@ -2150,34 +2150,32 @@
else:
assert rcs_type in (RevisionControlSystems.SVN,
RevisionControlSystems.BZR_SVN)
- code_import = code_import_set.new(
+ return code_import_set.new(
registrant, target, branch_name, rcs_type=rcs_type,
- url=svn_branch_url)
+ url=svn_branch_url, review_status=review_status)
elif git_repo_url is not None:
assert rcs_type in (None, RevisionControlSystems.GIT)
- code_import = code_import_set.new(
+ return code_import_set.new(
registrant, target, branch_name,
rcs_type=RevisionControlSystems.GIT,
- url=git_repo_url)
+ url=git_repo_url, review_status=review_status)
elif hg_repo_url is not None:
- code_import = code_import_set.new(
+ return code_import_set.new(
registrant, target, branch_name,
rcs_type=RevisionControlSystems.HG,
- url=hg_repo_url)
+ url=hg_repo_url, review_status=review_status)
elif bzr_branch_url is not None:
- code_import = code_import_set.new(
+ return code_import_set.new(
registrant, target, branch_name,
rcs_type=RevisionControlSystems.BZR,
- url=bzr_branch_url)
+ url=bzr_branch_url, review_status=review_status)
else:
assert rcs_type in (None, RevisionControlSystems.CVS)
- code_import = code_import_set.new(
+ return code_import_set.new(
registrant, target, branch_name,
rcs_type=RevisionControlSystems.CVS,
- cvs_root=cvs_root, cvs_module=cvs_module)
- if review_status:
- removeSecurityProxy(code_import).review_status = review_status
- return code_import
+ cvs_root=cvs_root, cvs_module=cvs_module,
+ review_status=review_status)
def makeChangelog(self, spn=None, versions=[]):
"""Create and return a LFA of a valid Debian-style changelog."""
=== modified file 'utilities/sourcedeps.cache'
--- utilities/sourcedeps.cache 2011-08-19 14:03:25 +0000
+++ utilities/sourcedeps.cache 2011-08-28 19:23:29 +0000
@@ -1,8 +1,4 @@
{
- "bzr-builder": [
- 68,
- "launchpad@xxxxxxxxxxxxxxxxx-20101123183213-777lz46xgagn1deg"
- ],
"testresources": [
16,
"robertc@xxxxxxxxxxxxxxxxx-20050911111209-ee5da49011cf936a"
@@ -31,14 +27,18 @@
24,
"launchpad@xxxxxxxxxxxxxxxxx-20100601182722-wo7h2fh0fvyw3aaq"
],
- "lpreview": [
- 23,
- "launchpad@xxxxxxxxxxxxxxxxx-20090720061538-euyh68ifavhy0pi8"
- ],
"bzr-git": [
259,
"launchpad@xxxxxxxxxxxxxxxxx-20110601140035-gl5merbechngjw5s"
],
+ "loggerhead": [
+ 454,
+ "gavin@xxxxxxxxxxx-20110811101303-ka12yvnq2p48e2t8"
+ ],
+ "bzr-builder": [
+ 68,
+ "launchpad@xxxxxxxxxxxxxxxxx-20101123183213-777lz46xgagn1deg"
+ ],
"bzr-loom": [
49,
"launchpad@xxxxxxxxxxxxxxxxx-20110601122412-54vo3k8yae9i2zve"
@@ -47,9 +47,9 @@
4,
"sinzui-20090526164636-1swugzupwvjgomo4"
],
- "loggerhead": [
- 452,
- "andrew.bennetts@xxxxxxxxxxxxx-20110628173100-owrifrnckvoi60af"
+ "lpreview": [
+ 23,
+ "launchpad@xxxxxxxxxxxxxxxxx-20090720061538-euyh68ifavhy0pi8"
],
"difftacular": [
6,
