launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21098
[Merge] lp:~cjwatson/launchpad/codeimport-git-webservice into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-webservice into lp:launchpad with lp:~cjwatson/launchpad/codeimport-git-edit-views as a prerequisite.
Commit message:
Allow creating Git-targeted code imports on the webservice.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1469459 in Launchpad itself: "import external code into a LP git repo (natively)"
https://bugs.launchpad.net/launchpad/+bug/1469459
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-webservice/+merge/308399
The branch_name parameter is slightly unfortunate now, but it's probably better than introducing a new endpoint or having a mostly-duplicate repository_name parameter.
I need to do penance for not unittestifying the doctests, but I ran out of time.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-webservice into lp:launchpad.
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2016-10-05 09:29:41 +0000
+++ lib/lp/code/errors.py 2016-10-13 15:58:50 +0000
@@ -27,7 +27,9 @@
'CannotUpgradeNonHosted',
'CodeImportAlreadyRequested',
'CodeImportAlreadyRunning',
+ 'CodeImportInvalidTargetType',
'CodeImportNotInReviewedState',
+ 'CodeImportGitTargetFeatureDisabled',
'ClaimReviewFailed',
'DiffNotFound',
'GitDefaultConflict',
@@ -488,6 +490,25 @@
@error_status(httplib.BAD_REQUEST)
+class CodeImportInvalidTargetType(Exception):
+ """Raised for code imports with an invalid target for their type."""
+
+ def __init__(self, target, target_rcs_type):
+ super(CodeImportInvalidTargetType, self).__init__(
+ "Objects of type %s do not support code imports targeting %s." %
+ (target.__class__.__name__, target_rcs_type))
+
+
+@error_status(httplib.UNAUTHORIZED)
+class CodeImportGitTargetFeatureDisabled(Exception):
+ """Only certain users can create new Git-targeted code imports."""
+
+ def __init__(self):
+ super(CodeImportGitTargetFeatureDisabled, self).__init__(
+ "You do not have permission to create Git-targeted code imports.")
+
+
+@error_status(httplib.BAD_REQUEST)
class TooNewRecipeFormat(Exception):
"""The format of the recipe supplied was too new."""
=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py 2016-10-13 15:58:50 +0000
+++ lib/lp/code/interfaces/codeimport.py 2016-10-13 15:58:50 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
__all__ = [
+ 'CODE_IMPORT_GIT_TARGET_FEATURE_FLAG',
'ICodeImport',
'ICodeImportSet',
]
@@ -51,6 +52,9 @@
)
+CODE_IMPORT_GIT_TARGET_FEATURE_FLAG = u'code.import.git_target'
+
+
def validate_cvs_root(cvsroot):
try:
root = CVSRoot(cvsroot)
=== modified file 'lib/lp/code/interfaces/hasbranches.py'
--- lib/lp/code/interfaces/hasbranches.py 2016-09-07 02:46:20 +0000
+++ lib/lp/code/interfaces/hasbranches.py 2016-10-13 15:58:50 +0000
@@ -7,6 +7,8 @@
__all__ = [
'IHasBranches',
'IHasCodeImports',
+ 'IHasCodeImportsToBazaar',
+ 'IHasCodeImportsToGit',
'IHasMergeProposals',
'IHasRequestedReviews',
]
@@ -35,6 +37,7 @@
BranchLifecycleStatus,
BranchMergeProposalStatus,
RevisionControlSystems,
+ TargetRevisionControlSystems,
)
@@ -154,13 +157,14 @@
This interface defines the common methods that for working with them.
"""
- # In order to minimise dependancies the returns_collection is defined as
+ # In order to minimise dependencies the returns_collection is defined as
# Interface here and defined fully in the circular imports file.
@operation_parameters(
branch_name=TextLine(
title=_('Name of branch to create'), required=True),
rcs_type=Choice(vocabulary=RevisionControlSystems, required=True),
+ target_rcs_type=Choice(vocabulary=TargetRevisionControlSystems),
url=TextLine(title=_('Foreign VCS URL')),
cvs_root=TextLine(title=_('CVS root URL')),
cvs_module=TextLine(title=_('CVS module to import')),
@@ -170,19 +174,31 @@
@call_with(registrant=REQUEST_USER)
@export_factory_operation(Interface, []) # Really ICodeImport.
@operation_for_version('beta')
- def newCodeImport(registrant=None, branch_name=None, rcs_type=None,
- url=None, cvs_root=None, cvs_module=None, owner=None):
+ def newCodeImport(registrant=None, branch_name=None,
+ rcs_type=None, target_rcs_type=None, url=None,
+ cvs_root=None, cvs_module=None, owner=None):
"""Create a new code import.
:param registrant: The IPerson to record as the registrant of the
- import
- :param branch_name: The name of the branch to create.
+ import.
+ :param branch_name: The name of the branch or repository to create.
:param rcs_type: The type of the foreign VCS.
+ :param target_rcs_type: The type of the branch or repository to
+ create (Bazaar or Git).
:param url: The URL to import from if the VCS type uses a single URL
(i.e. isn't CVS).
:param cvs_root: The CVSROOT for a CVS import.
:param cvs_module: The module to import for a CVS import.
- :param owner: Who should own the created branch, or None for it to
- be the same as the registrant, or the caller over the API.
+ :param owner: Who should own the created branch or repository, or
+ None for it to be the same as the registrant, or the caller over
+ the API.
:returns: An instance of `ICodeImport`.
"""
+
+
+class IHasCodeImportsToBazaar(IHasCodeImports):
+ """Marker interface for targets that support code imports to Bazaar."""
+
+
+class IHasCodeImportsToGit(IHasCodeImports):
+ """Marker interface for targets that support code imports to Git."""
=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py 2016-10-13 15:58:50 +0000
+++ lib/lp/code/model/codeimport.py 2016-10-13 15:58:50 +0000
@@ -49,6 +49,7 @@
from lp.code.errors import (
CodeImportAlreadyRequested,
CodeImportAlreadyRunning,
+ CodeImportInvalidTargetType,
CodeImportNotInReviewedState,
)
from lp.code.interfaces.branch import IBranch
@@ -62,6 +63,10 @@
from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitnamespace import get_git_namespace
from lp.code.interfaces.gitrepository import IGitRepository
+from lp.code.interfaces.hasbranches import (
+ IHasCodeImportsToBazaar,
+ IHasCodeImportsToGit,
+ )
from lp.code.mail.codeimport import code_import_updated
from lp.code.model.codeimportjob import CodeImportJobWorkflow
from lp.code.model.codeimportresult import CodeImportResult
@@ -291,9 +296,13 @@
if owner is None:
owner = registrant
if target_rcs_type == TargetRevisionControlSystems.BZR:
+ if not IHasCodeImportsToBazaar.providedBy(context):
+ raise CodeImportInvalidTargetType(context, target_rcs_type)
target = IBranchTarget(context)
namespace = target.getNamespace(owner)
elif target_rcs_type == TargetRevisionControlSystems.GIT:
+ if not IHasCodeImportsToGit.providedBy(context):
+ raise CodeImportInvalidTargetType(context, target_rcs_type)
if rcs_type != RevisionControlSystems.GIT:
raise AssertionError(
"Can't import rcs_type %s into a Git repository" %
=== modified file 'lib/lp/code/model/hasbranches.py'
--- lib/lp/code/model/hasbranches.py 2016-10-03 17:00:56 +0000
+++ lib/lp/code/model/hasbranches.py 2016-10-13 15:58:50 +0000
@@ -20,18 +20,23 @@
BranchMergeProposalStatus,
TargetRevisionControlSystems,
)
+from lp.code.errors import CodeImportGitTargetFeatureDisabled
from lp.code.interfaces.branch import DEFAULT_BRANCH_STATUS_IN_LISTING
from lp.code.interfaces.branchcollection import (
IAllBranches,
IBranchCollection,
)
-from lp.code.interfaces.codeimport import ICodeImportSet
+from lp.code.interfaces.codeimport import (
+ CODE_IMPORT_GIT_TARGET_FEATURE_FLAG,
+ ICodeImportSet,
+ )
from lp.code.interfaces.gitcollection import (
IAllGitRepositories,
IGitCollection,
)
from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.services.database.decoratedresultset import DecoratedResultSet
+from lp.services.features import getFeatureFlag
class HasBranchesMixin:
@@ -129,10 +134,14 @@
class HasCodeImportsMixin:
def newCodeImport(self, registrant=None, branch_name=None,
- rcs_type=None, url=None, cvs_root=None, cvs_module=None,
- owner=None):
+ rcs_type=None, target_rcs_type=None, url=None,
+ cvs_root=None, cvs_module=None, owner=None):
"""See `IHasCodeImports`."""
+ if target_rcs_type is None:
+ target_rcs_type = TargetRevisionControlSystems.BZR
+ if (target_rcs_type == TargetRevisionControlSystems.GIT and
+ not getFeatureFlag(CODE_IMPORT_GIT_TARGET_FEATURE_FLAG)):
+ raise CodeImportGitTargetFeatureDisabled
return getUtility(ICodeImportSet).new(
- registrant, self, branch_name, rcs_type,
- TargetRevisionControlSystems.BZR, url=url,
- cvs_root=cvs_root, cvs_module=cvs_module, owner=owner)
+ registrant, self, branch_name, rcs_type, target_rcs_type,
+ url=url, cvs_root=cvs_root, cvs_module=cvs_module, owner=owner)
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py 2016-10-12 12:51:05 +0000
+++ lib/lp/code/model/tests/test_codeimport.py 2016-10-13 15:58:50 +0000
@@ -30,6 +30,7 @@
BranchCreatorNotMemberOfOwnerTeam,
CodeImportAlreadyRequested,
CodeImportAlreadyRunning,
+ CodeImportInvalidTargetType,
CodeImportNotInReviewedState,
GitRepositoryCreatorNotMemberOfOwnerTeam,
)
@@ -218,7 +219,7 @@
def test_junk_code_import_rejected(self):
"""You are not allowed to create code imports targetting +junk."""
registrant = self.factory.makePerson()
- self.assertRaises(AssertionError, CodeImportSet().new,
+ self.assertRaises(CodeImportInvalidTargetType, CodeImportSet().new,
registrant=registrant,
context=registrant,
branch_name=u'imported',
=== modified file 'lib/lp/code/stories/webservice/xx-code-import.txt'
--- lib/lp/code/stories/webservice/xx-code-import.txt 2014-02-25 01:48:48 +0000
+++ lib/lp/code/stories/webservice/xx-code-import.txt 2016-10-13 15:58:50 +0000
@@ -5,8 +5,9 @@
if it is an import branch.
>>> from zope.security.proxy import removeSecurityProxy
+ >>> from lp.code.tests.helpers import GitHostingFixture
+ >>> from lp.services.webapp.interfaces import OAuthPermission
>>> from lp.testing.pages import webservice_for_person
- >>> from lp.services.webapp.interfaces import OAuthPermission
First we create some objects for use in the tests.
@@ -59,6 +60,8 @@
Reviewed
>>> print representation['rcs_type']
Subversion via bzr-svn
+ >>> print representation['target_rcs_type']
+ Bazaar
>>> print representation['url']
http://svn.domain.com/source
>>> print representation['cvs_root']
@@ -109,6 +112,8 @@
Reviewed
>>> print representation['rcs_type']
Subversion via bzr-svn
+ >>> print representation['target_rcs_type']
+ Bazaar
>>> print representation['url']
http://svn.domain.com/package_source
>>> print representation['cvs_root']
@@ -119,7 +124,8 @@
None
-== Creating Imports ==
+Creating Imports
+----------------
We can create an import using the API by calling a method on the project.
@@ -137,8 +143,12 @@
http://.../~import-owner/scruff/new-import/+code-import
>>> print representation['branch_link']
http://.../~import-owner/scruff/new-import
+ >>> print representation['git_repository_link']
+ None
>>> print representation['rcs_type']
Git
+ >>> print representation['target_rcs_type']
+ Bazaar
>>> print representation['url'] == new_remote_url
True
>>> print representation['cvs_root']
@@ -164,8 +174,12 @@
http://.../~import-owner/scruff/cvs-import/+code-import
>>> print representation['branch_link']
http://.../~import-owner/scruff/cvs-import
+ >>> print representation['git_repository_link']
+ None
>>> print representation['rcs_type']
Concurrent Versions System
+ >>> print representation['target_rcs_type']
+ Bazaar
>>> print representation['url']
None
>>> print representation['cvs_root'] == new_remote_url
@@ -175,6 +189,51 @@
>>> print representation['date_last_successful']
None
+We can create a Git-to-Git import, provided that we have the feature flag.
+
+ >>> from lp.code.interfaces.codeimport import (
+ ... CODE_IMPORT_GIT_TARGET_FEATURE_FLAG,
+ ... )
+ >>> from lp.services.features.testing import FeatureFixture
+
+ >>> product_url = '/' + product_name
+ >>> new_remote_url = factory.getUniqueURL()
+ >>> response = import_webservice.named_post(
+ ... product_url, 'newCodeImport', branch_name='new-import',
+ ... rcs_type='Git', target_rcs_type='Git', url=new_remote_url)
+ >>> print response.status
+ 401
+ >>> print response.body
+ You do not have permission to create Git-targeted code imports.
+ >>> with FeatureFixture({CODE_IMPORT_GIT_TARGET_FEATURE_FLAG: u'on'}):
+ ... with GitHostingFixture():
+ ... response = import_webservice.named_post(
+ ... product_url, 'newCodeImport', branch_name='new-import',
+ ... rcs_type='Git', target_rcs_type='Git', url=new_remote_url)
+ >>> print response.status
+ 201
+ >>> location = response.getHeader('Location')
+ >>> response = import_webservice.get(location)
+ >>> representation = response.jsonBody()
+ >>> print representation['self_link']
+ http://.../~import-owner/scruff/+git/new-import/+code-import
+ >>> print representation['branch_link']
+ None
+ >>> print representation['git_repository_link']
+ http://.../~import-owner/scruff/+git/new-import
+ >>> print representation['rcs_type']
+ Git
+ >>> print representation['target_rcs_type']
+ Git
+ >>> print representation['url'] == new_remote_url
+ True
+ >>> print representation['cvs_root']
+ None
+ >>> print representation['cvs_module']
+ None
+ >>> print representation['date_last_successful']
+ None
+
We can also create an import targetting a source package.
>>> source_package_url = (
@@ -193,7 +252,46 @@
http://.../~import-owner/scruffbuntu/manic/scruff/new-import/+code-import
>>> print representation['branch_link']
http://.../~import-owner/scruffbuntu/manic/scruff/new-import
- >>> print representation['rcs_type']
+ >>> print representation['git_repository_link']
+ None
+ >>> print representation['rcs_type']
+ Git
+ >>> print representation['target_rcs_type']
+ Bazaar
+ >>> print representation['url'] == new_remote_url
+ True
+ >>> print representation['cvs_root']
+ None
+ >>> print representation['cvs_module']
+ None
+ >>> print representation['date_last_successful']
+ None
+
+We can create a Git-to-Git import targetting a distribution source package.
+
+ >>> distro_source_package_url = (
+ ... '/' + distribution.name + '/+source/' + source_package.name)
+ >>> new_remote_url = factory.getUniqueURL()
+ >>> with FeatureFixture({CODE_IMPORT_GIT_TARGET_FEATURE_FLAG: u'on'}):
+ ... with GitHostingFixture():
+ ... response = import_webservice.named_post(
+ ... distro_source_package_url, 'newCodeImport',
+ ... branch_name='new-import', rcs_type='Git',
+ ... target_rcs_type='Git', url=new_remote_url)
+ >>> print response.status
+ 201
+ >>> location = response.getHeader('Location')
+ >>> response = import_webservice.get(location)
+ >>> representation = response.jsonBody()
+ >>> print representation['self_link']
+ http://.../~import-owner/scruffbuntu/+source/scruff/+git/new-import/+code-import
+ >>> print representation['branch_link']
+ None
+ >>> print representation['git_repository_link']
+ http://.../~import-owner/scruffbuntu/+source/scruff/+git/new-import
+ >>> print representation['rcs_type']
+ Git
+ >>> print representation['target_rcs_type']
Git
>>> print representation['url'] == new_remote_url
True
@@ -220,8 +318,12 @@
http://.../~import-owner-team/scruff/team-import/+code-import
>>> print representation['branch_link']
http://.../~import-owner-team/scruff/team-import
+ >>> print representation['git_repository_link']
+ None
>>> print representation['rcs_type']
Git
+ >>> print representation['target_rcs_type']
+ Bazaar
>>> print representation['url'] == new_remote_url
True
>>> print representation['cvs_root']
@@ -232,7 +334,8 @@
None
-== Requesting an Import ==
+Requesting an Import
+--------------------
You can request that an approved, working import happen soon over the
API using the requestImport() method.
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2016-04-13 13:28:10 +0000
+++ lib/lp/registry/configure.zcml 2016-10-13 15:58:50 +0000
@@ -607,6 +607,11 @@
<allow
interface="lp.code.interfaces.hasgitrepositories.IHasGitRepositories" />
+
+ <!-- IHasCodeImports -->
+
+ <allow
+ interface="lp.code.interfaces.hasbranches.IHasCodeImports" />
</class>
<adapter
provides="lp.registry.interfaces.distribution.IDistribution"
=== modified file 'lib/lp/registry/interfaces/distributionsourcepackage.py'
--- lib/lp/registry/interfaces/distributionsourcepackage.py 2015-10-12 16:16:28 +0000
+++ lib/lp/registry/interfaces/distributionsourcepackage.py 2016-10-13 15:58:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Source package in Distribution interfaces."""
@@ -32,6 +32,7 @@
)
from lp.code.interfaces.hasbranches import (
IHasBranches,
+ IHasCodeImportsToGit,
IHasMergeProposals,
)
from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
@@ -44,7 +45,7 @@
IHasMergeProposals, IHasOfficialBugTags,
IStructuralSubscriptionTarget,
IQuestionTarget, IHasDrivers,
- IHasGitRepositories):
+ IHasGitRepositories, IHasCodeImportsToGit):
"""Represents a source package in a distribution.
Create IDistributionSourcePackages by invoking
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2015-10-01 17:32:41 +0000
+++ lib/lp/registry/interfaces/product.py 2016-10-13 15:58:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces including and related to IProduct."""
@@ -100,7 +100,8 @@
)
from lp.code.interfaces.hasbranches import (
IHasBranches,
- IHasCodeImports,
+ IHasCodeImportsToBazaar,
+ IHasCodeImportsToGit,
IHasMergeProposals,
)
from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
@@ -484,7 +485,8 @@
IHasMugshot, IHasSprints, IHasTranslationImports,
ITranslationPolicy, IKarmaContext, IMakesAnnouncements,
IOfficialBugTagTargetPublic, IHasOOPSReferences,
- IHasRecipes, IHasCodeImports, IServiceUsage, IHasGitRepositories):
+ IHasRecipes, IHasCodeImportsToBazaar, IHasCodeImportsToGit,
+ IServiceUsage, IHasGitRepositories):
"""Public IProduct properties."""
registrant = exported(
=== modified file 'lib/lp/registry/interfaces/sourcepackage.py'
--- lib/lp/registry/interfaces/sourcepackage.py 2016-06-01 01:59:32 +0000
+++ lib/lp/registry/interfaces/sourcepackage.py 2016-10-13 15:58:50 +0000
@@ -51,7 +51,7 @@
)
from lp.code.interfaces.hasbranches import (
IHasBranches,
- IHasCodeImports,
+ IHasCodeImportsToBazaar,
IHasMergeProposals,
)
from lp.registry.interfaces.productseries import IProductSeries
@@ -69,7 +69,7 @@
class ISourcePackagePublic(IBugTarget, IHasBranches, IHasMergeProposals,
- IHasOfficialBugTags, IHasCodeImports,
+ IHasOfficialBugTags, IHasCodeImportsToBazaar,
IHasTranslationImports, IHasTranslationTemplates,
IHasDrivers, IHasOwner):
"""Public attributes for SourcePackage."""
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2015-10-12 16:16:28 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2016-10-13 15:58:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Classes to represent source packages in a distribution."""
@@ -41,6 +41,7 @@
)
from lp.code.model.hasbranches import (
HasBranchesMixin,
+ HasCodeImportsMixin,
HasMergeProposalsMixin,
)
from lp.registry.interfaces.distributionsourcepackage import (
@@ -119,6 +120,7 @@
SourcePackageQuestionTargetMixin,
StructuralSubscriptionTargetMixin,
HasBranchesMixin,
+ HasCodeImportsMixin,
HasCustomLanguageCodesMixin,
HasMergeProposalsMixin,
HasDriversMixin):
Follow ups
