launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22246
[Merge] lp:~cjwatson/launchpad/decouple-code-import-worker-arguments into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/decouple-code-import-worker-arguments into lp:launchpad.
Commit message:
Move code import worker argument construction into CodeImportJob.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/decouple-code-import-worker-arguments/+merge/341484
This can only be used from the main application side anyway because it talks directly to the database, so it's much less confusing to have this in the main application code. Moving this will also help us to split codeimport out to a separate codebase.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/decouple-code-import-worker-arguments into lp:launchpad.
=== modified file 'lib/lp/code/interfaces/codeimportjob.py'
--- lib/lp/code/interfaces/codeimportjob.py 2013-01-07 02:40:55 +0000
+++ lib/lp/code/interfaces/codeimportjob.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces and enumeratrions for CodeImportJobs.
@@ -97,6 +97,9 @@
to the time of the current transaction.
"""
+ def makeWorkerArguments():
+ """Return a list of worker arguments for this job."""
+
class ICodeImportJobSet(Interface):
"""The set of pending and active code import jobs."""
=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py 2016-10-20 09:34:02 +0000
+++ lib/lp/code/model/codeimportjob.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database classes for the CodeImportJob table."""
@@ -31,6 +31,12 @@
CodeImportMachineState,
CodeImportResultStatus,
CodeImportReviewStatus,
+ RevisionControlSystems,
+ )
+from lp.code.interfaces.branch import IBranch
+from lp.code.interfaces.codehosting import (
+ branch_id_alias,
+ compose_public_url,
)
from lp.code.interfaces.codeimportevent import ICodeImportEventSet
from lp.code.interfaces.codeimportjob import (
@@ -104,6 +110,61 @@
"id = %s AND date_due <= %s" % sqlvalues(self.id, UTC_NOW))
return import_job is not None
+ def makeWorkerArguments(self):
+ """See `ICodeImportJob`."""
+ # Keep this in sync with CodeImportSourceDetails.fromArguments.
+ code_import = self.code_import
+ target = code_import.target
+
+ if IBranch.providedBy(target):
+ target_id = target.id
+ else:
+ # We don't have a better way to identify the target repository
+ # than the mutable unique name, but the macaroon constrains
+ # pushes tightly enough that the worst case is an authentication
+ # failure.
+ target_id = target.unique_name
+
+ if code_import.rcs_type == RevisionControlSystems.BZR_SVN:
+ rcs_type = 'bzr-svn'
+ target_rcs_type = 'bzr'
+ elif code_import.rcs_type == RevisionControlSystems.CVS:
+ rcs_type = 'cvs'
+ target_rcs_type = 'bzr'
+ elif code_import.rcs_type == RevisionControlSystems.GIT:
+ rcs_type = 'git'
+ if IBranch.providedBy(target):
+ target_rcs_type = 'bzr'
+ else:
+ target_rcs_type = 'git'
+ elif code_import.rcs_type == RevisionControlSystems.BZR:
+ rcs_type = 'bzr'
+ target_rcs_type = 'bzr'
+ else:
+ raise AssertionError("Unknown rcs_type %r." % code_import.rcs_type)
+
+ result = [str(target_id), '%s:%s' % (rcs_type, target_rcs_type)]
+ if rcs_type in ('bzr-svn', 'git', 'bzr'):
+ result.append(str(code_import.url))
+ if (IBranch.providedBy(target) and
+ target.stacked_on is not None and
+ not target.stacked_on.private):
+ stacked_path = branch_id_alias(target.stacked_on)
+ stacked_on_url = compose_public_url('http', stacked_path)
+ result.append(stacked_on_url)
+ elif rcs_type == 'cvs':
+ result.append(str(code_import.cvs_root))
+ result.append(str(code_import.cvs_module))
+ else:
+ raise AssertionError("Unknown rcs_type %r." % rcs_type)
+ if target_rcs_type == 'git':
+ issuer = getUtility(IMacaroonIssuer, 'code-import-job')
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(self)
+ # XXX cjwatson 2016-10-12: Consider arranging for this to be
+ # passed to worker processes in the environment instead.
+ result.append(macaroon.serialize())
+ return result
+
@implementer(ICodeImportJobSet, ICodeImportJobSetPublic)
class CodeImportJobSet(object):
=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py 2018-01-02 16:10:26 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for CodeImportJob and CodeImportJobWorkflow."""
@@ -17,13 +17,17 @@
from pymacaroons import Macaroon
from pytz import UTC
from testtools.matchers import (
+ Equals,
+ Matcher,
MatchesListwise,
MatchesStructure,
+ Mismatch,
)
import transaction
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from lp.app.enums import InformationType
from lp.code.enums import (
CodeImportEventType,
CodeImportJobState,
@@ -31,6 +35,10 @@
CodeImportReviewStatus,
TargetRevisionControlSystems,
)
+from lp.code.interfaces.codehosting import (
+ branch_id_alias,
+ compose_public_url,
+ )
from lp.code.interfaces.codeimport import ICodeImportSet
from lp.code.interfaces.codeimportevent import ICodeImportEventSet
from lp.code.interfaces.codeimportjob import (
@@ -76,6 +84,106 @@
return login_celebrity('vcs_imports')
+class CodeImportJobMacaroonVerifies(Matcher):
+ """Matches if a code-import-job macaroon can be verified."""
+
+ def __init__(self, code_import):
+ self.code_import = code_import
+
+ def match(self, macaroon_raw):
+ issuer = getUtility(IMacaroonIssuer, 'code-import-job')
+ macaroon = Macaroon.deserialize(macaroon_raw)
+ if not issuer.verifyMacaroon(macaroon, self.code_import.import_job):
+ return Mismatch("Macaroon '%s' does not verify" % macaroon_raw)
+
+
+class TestCodeImportJob(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestCodeImportJob, self).setUp()
+ login_for_code_imports()
+
+ def assertArgumentsMatch(self, code_import, matcher, start_job=False):
+ job = self.factory.makeCodeImportJob(code_import=code_import)
+ if start_job:
+ machine = self.factory.makeCodeImportMachine(set_online=True)
+ getUtility(ICodeImportJobWorkflow).startJob(job, machine)
+ self.assertThat(job.makeWorkerArguments(), matcher)
+
+ def test_bzr_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ bzr_branch_url="http://example.com/foo";)
+ self.assertArgumentsMatch(
+ code_import, Equals([
+ str(code_import.branch.id), 'bzr:bzr',
+ 'http://example.com/foo']))
+
+ def test_git_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ git_repo_url="git://git.example.com/project.git")
+ self.assertArgumentsMatch(
+ code_import, Equals([
+ str(code_import.branch.id), 'git:bzr',
+ 'git://git.example.com/project.git']))
+
+ def test_git_to_git_arguments(self):
+ self.pushConfig('codeimport', macaroon_secret_key='some-secret')
+ self.useFixture(GitHostingFixture())
+ code_import = self.factory.makeCodeImport(
+ git_repo_url="git://git.example.com/project.git",
+ target_rcs_type=TargetRevisionControlSystems.GIT)
+ self.assertArgumentsMatch(
+ code_import, MatchesListwise([
+ Equals(code_import.git_repository.unique_name),
+ Equals('git:git'), Equals('git://git.example.com/project.git'),
+ CodeImportJobMacaroonVerifies(code_import)]),
+ # Start the job so that the macaroon can be verified.
+ start_job=True)
+
+ def test_cvs_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ cvs_root=':pserver:foo@xxxxxxxxxxx/bar', cvs_module='bar')
+ self.assertArgumentsMatch(
+ code_import, Equals([
+ str(code_import.branch.id), 'cvs:bzr',
+ ':pserver:foo@xxxxxxxxxxx/bar', 'bar']))
+
+ def test_bzr_svn_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ svn_branch_url='svn://svn.example.com/trunk')
+ self.assertArgumentsMatch(
+ code_import, Equals([
+ str(code_import.branch.id), 'bzr-svn:bzr',
+ 'svn://svn.example.com/trunk']))
+
+ def test_bzr_stacked(self):
+ devfocus = self.factory.makeAnyBranch()
+ code_import = self.factory.makeCodeImport(
+ bzr_branch_url='bzr://bzr.example.com/foo',
+ context=devfocus.target.context)
+ code_import.branch.stacked_on = devfocus
+ self.assertArgumentsMatch(
+ code_import, Equals([
+ str(code_import.branch.id), 'bzr:bzr',
+ 'bzr://bzr.example.com/foo',
+ compose_public_url('http', branch_id_alias(devfocus))]))
+
+ def test_bzr_stacked_private(self):
+ # Code imports can't be stacked on private branches.
+ devfocus = self.factory.makeAnyBranch(
+ information_type=InformationType.USERDATA)
+ code_import = self.factory.makeCodeImport(
+ context=removeSecurityProxy(devfocus).target.context,
+ bzr_branch_url='bzr://bzr.example.com/foo')
+ branch = removeSecurityProxy(code_import.branch)
+ branch.stacked_on = devfocus
+ self.assertArgumentsMatch(
+ code_import, Equals([
+ str(branch.id), 'bzr:bzr', 'bzr://bzr.example.com/foo']))
+
+
class TestCodeImportJobSet(TestCaseWithFactory):
"""Unit tests for the CodeImportJobSet utility."""
=== modified file 'lib/lp/code/xmlrpc/codeimportscheduler.py'
--- lib/lp/code/xmlrpc/codeimportscheduler.py 2016-10-11 13:41:32 +0000
+++ lib/lp/code/xmlrpc/codeimportscheduler.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""The code import scheduler XML-RPC API."""
@@ -18,7 +18,6 @@
ICodeImportJobWorkflow,
)
from lp.code.interfaces.codeimportscheduler import ICodeImportScheduler
-from lp.codehosting.codeimport.worker import CodeImportSourceDetails
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.webapp import (
canonical_url,
@@ -67,8 +66,7 @@
@return_fault
def _getImportDataForJobID(self, job_id):
job = self._getJob(job_id)
- arguments = CodeImportSourceDetails.fromCodeImportJob(
- job).asArguments()
+ arguments = job.makeWorkerArguments()
target = job.code_import.target
target_url = canonical_url(target)
log_file_name = '%s.log' % target.unique_name[1:].replace('/', '-')
=== modified file 'lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py'
--- lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py 2016-10-11 13:41:32 +0000
+++ lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test for the methods of `ICodeImportScheduler`."""
@@ -15,7 +15,6 @@
from lp.code.model.codeimportjob import CodeImportJob
from lp.code.tests.codeimporthelpers import make_running_import
from lp.code.xmlrpc.codeimportscheduler import CodeImportSchedulerAPI
-from lp.codehosting.codeimport.worker import CodeImportSourceDetails
from lp.services.database.constants import UTC_NOW
from lp.services.webapp import canonical_url
from lp.testing import (
@@ -63,8 +62,7 @@
code_import = removeSecurityProxy(code_import_job).code_import
code_import_arguments, target_url, log_file_name = \
self.api.getImportDataForJobID(code_import_job.id)
- import_as_arguments = CodeImportSourceDetails.fromCodeImportJob(
- code_import_job).asArguments()
+ import_as_arguments = code_import_job.makeWorkerArguments()
expected_log_file_name = '%s.log' % (
code_import.target.unique_name[1:].replace('/', '-'))
self.assertEqual(
=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py 2018-02-14 01:27:28 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the code import worker."""
@@ -45,26 +45,10 @@
)
from dulwich.repo import Repo as GitRepo
from fixtures import FakeLogger
-from pymacaroons import Macaroon
import subvertpy
import subvertpy.client
import subvertpy.ra
-from testtools.matchers import (
- Equals,
- Matcher,
- MatchesListwise,
- Mismatch,
- )
-from zope.component import getUtility
-from lp.app.enums import InformationType
-from lp.code.enums import TargetRevisionControlSystems
-from lp.code.interfaces.codehosting import (
- branch_id_alias,
- compose_public_url,
- )
-from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
-from lp.code.tests.helpers import GitHostingFixture
import lp.codehosting
from lp.codehosting.codeimport.tarball import (
create_tarball,
@@ -100,16 +84,8 @@
from lp.codehosting.tests.helpers import create_branch_with_one_revision
from lp.services.config import config
from lp.services.log.logger import BufferLogger
-from lp.services.macaroons.interfaces import IMacaroonIssuer
-from lp.testing import (
- celebrity_logged_in,
- TestCase,
- TestCaseWithFactory,
- )
-from lp.testing.layers import (
- BaseLayer,
- DatabaseFunctionalLayer,
- )
+from lp.testing import TestCase
+from lp.testing.layers import BaseLayer
class ForeignBranchPluginLayer(BaseLayer):
@@ -820,8 +796,8 @@
raise NotImplementedError(
"Override this with a VCS-specific implementation.")
- def makeSourceDetails(self, module_name, files, stacked_on_url=None):
- """Make a `CodeImportSourceDetails` that points to a real repository.
+ def makeWorkerArguments(self, module_name, files, stacked_on_url=None):
+ """Make a list of worker arguments pointing to a real repository.
This should set `self.foreign_commit_count` to an appropriate value.
@@ -830,6 +806,11 @@
raise NotImplementedError(
"Override this with a VCS-specific implementation.")
+ def makeSourceDetails(self, module_name, files, stacked_on_url=None):
+ """Make a `CodeImportSourceDetails` pointing to a real repository."""
+ return CodeImportSourceDetails.fromArguments(self.makeWorkerArguments(
+ module_name, files, stacked_on_url=stacked_on_url))
+
def getStoredBazaarBranch(self, worker):
"""Get the Bazaar branch 'worker' stored into its BazaarBranchStore.
"""
@@ -876,8 +857,9 @@
def test_import_script(self):
# Like test_import, but using the code-import-worker.py script
# to perform the import.
- source_details = self.makeSourceDetails(
+ arguments = self.makeWorkerArguments(
'trunk', [('README', 'Original contents')])
+ source_details = CodeImportSourceDetails.fromArguments(arguments)
clean_up_default_stores_for_import(source_details)
@@ -885,8 +867,7 @@
config.root, 'scripts', 'code-import-worker.py')
output = tempfile.TemporaryFile()
retcode = subprocess.call(
- [script_path, '--access-policy=anything'] +
- source_details.asArguments(),
+ [script_path, '--access-policy=anything'] + arguments,
stderr=output, stdout=output)
self.assertEqual(retcode, 0)
@@ -914,8 +895,9 @@
# with a code of CodeImportWorkerExitCode.SUCCESS. After a successful
# import that does not import revisions, the worker exits with a code
# of CodeImportWorkerExitCode.SUCCESS_NOCHANGE.
- source_details = self.makeSourceDetails(
+ arguments = self.makeWorkerArguments(
'trunk', [('README', 'Original contents')])
+ source_details = CodeImportSourceDetails.fromArguments(arguments)
clean_up_default_stores_for_import(source_details)
@@ -923,13 +905,11 @@
config.root, 'scripts', 'code-import-worker.py')
output = tempfile.TemporaryFile()
retcode = subprocess.call(
- [script_path, '--access-policy=anything'] +
- source_details.asArguments(),
+ [script_path, '--access-policy=anything'] + arguments,
stderr=output, stdout=output)
self.assertEqual(retcode, CodeImportWorkerExitCode.SUCCESS)
retcode = subprocess.call(
- [script_path, '--access-policy=anything'] +
- source_details.asArguments(),
+ [script_path, '--access-policy=anything'] + arguments,
stderr=output, stdout=output)
self.assertEqual(retcode, CodeImportWorkerExitCode.SUCCESS_NOCHANGE)
@@ -971,9 +951,8 @@
self.foreign_commit_count += 1
shutil.rmtree('working_dir')
- def makeSourceDetails(self, module_name, files, stacked_on_url=None):
- """Make a CVS `CodeImportSourceDetails` pointing at a real CVS repo.
- """
+ def makeWorkerArguments(self, module_name, files, stacked_on_url=None):
+ """Make CVS worker arguments pointing at a real CVS repo."""
cvs_server = CVSServer(self.makeTemporaryDirectory())
cvs_server.start_server()
self.addCleanup(cvs_server.stop_server)
@@ -982,9 +961,10 @@
self.foreign_commit_count = 2
- return self.factory.makeCodeImportSourceDetails(
- rcstype='cvs', cvs_root=cvs_server.getRoot(), cvs_module='trunk',
- stacked_on_url=stacked_on_url)
+ return [
+ str(self.factory.getUniqueInteger()), 'cvs:bzr',
+ cvs_server.getRoot(), 'trunk',
+ ]
class SubversionImportHelpers:
@@ -1009,9 +989,8 @@
self.foreign_commit_count += 1
shutil.rmtree('working_tree')
- def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
- """Make a SVN `CodeImportSourceDetails` pointing at a real SVN repo.
- """
+ def makeWorkerArguments(self, branch_name, files, stacked_on_url=None):
+ """Make SVN worker arguments pointing at a real SVN repo."""
svn_server = SubversionServer(self.makeTemporaryDirectory())
svn_server.start_server()
self.addCleanup(svn_server.stop_server)
@@ -1019,9 +998,13 @@
svn_branch_url = svn_server.makeBranch(branch_name, files)
svn_branch_url = svn_branch_url.replace('://localhost/', ':///')
self.foreign_commit_count = 2
- return self.factory.makeCodeImportSourceDetails(
- rcstype=self.rcstype, url=svn_branch_url,
- stacked_on_url=stacked_on_url)
+ arguments = [
+ str(self.factory.getUniqueInteger()), '%s:bzr' % self.rcstype,
+ svn_branch_url,
+ ]
+ if stacked_on_url is not None:
+ arguments.append(stacked_on_url)
+ return arguments
class PullingImportWorkerTests:
@@ -1171,9 +1154,8 @@
committer="Joe Random Hacker <joe@xxxxxxxxxxx>", ref=ref)
self.foreign_commit_count += 1
- def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
- """Make a Git `CodeImportSourceDetails` pointing at a real Git repo.
- """
+ def makeWorkerArguments(self, branch_name, files, stacked_on_url=None):
+ """Make Git worker arguments pointing at a real Git repo."""
repository_store = self.makeTemporaryDirectory()
git_server = GitServer(repository_store)
git_server.start_server()
@@ -1182,9 +1164,13 @@
git_server.makeRepo('source', files)
self.foreign_commit_count = 1
- return self.factory.makeCodeImportSourceDetails(
- rcstype='git', url=git_server.get_url('source'),
- stacked_on_url=stacked_on_url)
+ arguments = [
+ str(self.factory.getUniqueInteger()), 'git:bzr',
+ git_server.get_url('source'),
+ ]
+ if stacked_on_url is not None:
+ arguments.append(stacked_on_url)
+ return arguments
def test_non_master(self):
# non-master branches can be specified in the import URL.
@@ -1250,9 +1236,8 @@
committer="Joe Random Hacker <joe@xxxxxxxxxxx>")
self.foreign_commit_count += 1
- def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
- """Make Bzr `CodeImportSourceDetails` pointing at a real Bzr repo.
- """
+ def makeWorkerArguments(self, branch_name, files, stacked_on_url=None):
+ """Make Bzr worker arguments pointing at a real Bzr repo."""
repository_path = self.makeTemporaryDirectory()
bzr_server = BzrServer(repository_path)
bzr_server.start_server()
@@ -1261,9 +1246,13 @@
bzr_server.makeRepo(files)
self.foreign_commit_count = 1
- return self.factory.makeCodeImportSourceDetails(
- rcstype='bzr', url=bzr_server.get_url(),
- stacked_on_url=stacked_on_url)
+ arguments = [
+ str(self.factory.getUniqueInteger()), 'bzr:bzr',
+ bzr_server.get_url(),
+ ]
+ if stacked_on_url is not None:
+ arguments.append(stacked_on_url)
+ return arguments
def test_partial(self):
self.skipTest(
@@ -1416,105 +1405,3 @@
self.old_server.stop_server()
self.assertEqual(
CodeImportWorkerExitCode.FAILURE_INVALID, worker.run())
-
-
-class CodeImportJobMacaroonVerifies(Matcher):
- """Matches if a code-import-job macaroon can be verified."""
-
- def __init__(self, code_import):
- self.code_import = code_import
-
- def match(self, macaroon_raw):
- issuer = getUtility(IMacaroonIssuer, 'code-import-job')
- macaroon = Macaroon.deserialize(macaroon_raw)
- if not issuer.verifyMacaroon(macaroon, self.code_import.import_job):
- return Mismatch("Macaroon '%s' does not verify" % macaroon_raw)
-
-
-class CodeImportSourceDetailsTests(TestCaseWithFactory):
-
- layer = DatabaseFunctionalLayer
-
- def setUp(self):
- # Use an admin user as we aren't checking edit permissions here.
- TestCaseWithFactory.setUp(self, 'admin@xxxxxxxxxxxxx')
-
- def assertArgumentsMatch(self, code_import, matcher, start_job=False):
- job = self.factory.makeCodeImportJob(code_import=code_import)
- if start_job:
- machine = self.factory.makeCodeImportMachine(set_online=True)
- with celebrity_logged_in("vcs_imports"):
- getUtility(ICodeImportJobWorkflow).startJob(job, machine)
- details = CodeImportSourceDetails.fromCodeImportJob(job)
- self.assertThat(details.asArguments(), matcher)
-
- def test_bzr_arguments(self):
- code_import = self.factory.makeCodeImport(
- bzr_branch_url="http://example.com/foo";)
- self.assertArgumentsMatch(
- code_import, Equals([
- str(code_import.branch.id), 'bzr:bzr',
- 'http://example.com/foo']))
-
- def test_git_arguments(self):
- code_import = self.factory.makeCodeImport(
- git_repo_url="git://git.example.com/project.git")
- self.assertArgumentsMatch(
- code_import, Equals([
- str(code_import.branch.id), 'git:bzr',
- 'git://git.example.com/project.git']))
-
- def test_git_to_git_arguments(self):
- self.pushConfig('codeimport', macaroon_secret_key='some-secret')
- self.useFixture(GitHostingFixture())
- code_import = self.factory.makeCodeImport(
- git_repo_url="git://git.example.com/project.git",
- target_rcs_type=TargetRevisionControlSystems.GIT)
- self.assertArgumentsMatch(
- code_import, MatchesListwise([
- Equals(code_import.git_repository.unique_name),
- Equals('git:git'), Equals('git://git.example.com/project.git'),
- CodeImportJobMacaroonVerifies(code_import)]),
- # Start the job so that the macaroon can be verified.
- start_job=True)
-
- def test_cvs_arguments(self):
- code_import = self.factory.makeCodeImport(
- cvs_root=':pserver:foo@xxxxxxxxxxx/bar', cvs_module='bar')
- self.assertArgumentsMatch(
- code_import, Equals([
- str(code_import.branch.id), 'cvs:bzr',
- ':pserver:foo@xxxxxxxxxxx/bar', 'bar']))
-
- def test_bzr_svn_arguments(self):
- code_import = self.factory.makeCodeImport(
- svn_branch_url='svn://svn.example.com/trunk')
- self.assertArgumentsMatch(
- code_import, Equals([
- str(code_import.branch.id), 'bzr-svn:bzr',
- 'svn://svn.example.com/trunk']))
-
- def test_bzr_stacked(self):
- devfocus = self.factory.makeAnyBranch()
- code_import = self.factory.makeCodeImport(
- bzr_branch_url='bzr://bzr.example.com/foo',
- context=devfocus.target.context)
- code_import.branch.stacked_on = devfocus
- self.assertArgumentsMatch(
- code_import, Equals([
- str(code_import.branch.id), 'bzr:bzr',
- 'bzr://bzr.example.com/foo',
- compose_public_url('http', branch_id_alias(devfocus))]))
-
- def test_bzr_stacked_private(self):
- # Code imports can't be stacked on private branches.
- devfocus = self.factory.makeAnyBranch(
- information_type=InformationType.USERDATA)
- code_import = self.factory.makeCodeImport(
- context=devfocus.target.context,
- bzr_branch_url='bzr://bzr.example.com/foo')
- code_import.branch.stacked_on = devfocus
- self.assertArgumentsMatch(
- code_import, Equals([
- str(code_import.branch.id), 'bzr:bzr',
- 'bzr://bzr.example.com/foo']))
=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2017-12-19 17:22:44 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the CodeImportWorkerMonitor and related classes."""
@@ -34,6 +34,7 @@
from twisted.python import log
from twisted.web import xmlrpc
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from lp.code.enums import (
CodeImportResultStatus,
@@ -762,7 +763,8 @@
self.factory.makePerson())
job = getUtility(ICodeImportJobSet).getJobForMachine('machine', 10)
self.assertEqual(code_import, job.code_import)
- source_details = CodeImportSourceDetails.fromCodeImportJob(job)
+ source_details = CodeImportSourceDetails.fromArguments(
+ removeSecurityProxy(job.makeWorkerArguments()))
if IBranch.providedBy(code_import.target):
clean_up_default_stores_for_import(source_details)
self.addCleanup(clean_up_default_stores_for_import, source_details)
=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py 2017-05-16 11:53:28 +0000
+++ lib/lp/codehosting/codeimport/worker.py 2018-03-15 21:30:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""The code import worker. This imports code from foreign repositories."""
@@ -73,18 +73,8 @@
)
from pymacaroons import Macaroon
import SCM
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-from lp.code.enums import RevisionControlSystems
-from lp.code.interfaces.branch import (
- get_blacklisted_hostnames,
- IBranch,
- )
-from lp.code.interfaces.codehosting import (
- branch_id_alias,
- compose_public_url,
- )
+from lp.code.interfaces.branch import get_blacklisted_hostnames
from lp.codehosting.codeimport.foreigntree import CVSWorkingTree
from lp.codehosting.codeimport.tarball import (
create_tarball,
@@ -97,7 +87,6 @@
SafeBranchOpener,
)
from lp.services.config import config
-from lp.services.macaroons.interfaces import IMacaroonIssuer
from lp.services.propertycache import cachedproperty
from lp.services.timeout import urlfetch
from lp.services.utils import sanitise_urls
@@ -285,7 +274,7 @@
As the worker doesn't talk to the database, we don't use
`CodeImport` objects for this.
- The 'fromArguments' and 'asArguments' methods convert to and from a form
+ The 'fromArguments' method builds an instance of this class from a form
of the information suitable for passing around on executables' command
lines.
@@ -319,6 +308,8 @@
@classmethod
def fromArguments(cls, arguments):
"""Convert command line-style arguments to an instance."""
+ # Keep this in sync with CodeImportJob.makeWorkerArguments.
+ arguments = list(arguments)
target_id = arguments.pop(0)
rcstype = arguments.pop(0)
# XXX cjwatson 2016-10-12: Remove compatibility code once the
@@ -354,71 +345,6 @@
target_id, rcstype, target_rcstype, url, cvs_root, cvs_module,
stacked_on_url, macaroon)
- @classmethod
- def fromCodeImportJob(cls, job):
- """Convert a `CodeImportJob` to an instance."""
- code_import = job.code_import
- target = code_import.target
- if IBranch.providedBy(target):
- if target.stacked_on is not None and not target.stacked_on.private:
- stacked_path = branch_id_alias(target.stacked_on)
- stacked_on_url = compose_public_url('http', stacked_path)
- else:
- stacked_on_url = None
- target_id = target.id
- else:
- # We don't have a better way to identify the target repository
- # than the mutable unique name, but the macaroon constrains
- # pushes tightly enough that the worst case is an authentication
- # failure.
- target_id = target.unique_name
- if code_import.rcs_type == RevisionControlSystems.BZR_SVN:
- return cls(
- target_id, 'bzr-svn', 'bzr', str(code_import.url),
- stacked_on_url=stacked_on_url)
- elif code_import.rcs_type == RevisionControlSystems.CVS:
- return cls(
- target_id, 'cvs', 'bzr',
- cvs_root=str(code_import.cvs_root),
- cvs_module=str(code_import.cvs_module))
- elif code_import.rcs_type == RevisionControlSystems.GIT:
- if IBranch.providedBy(target):
- return cls(
- target_id, 'git', 'bzr', str(code_import.url),
- stacked_on_url=stacked_on_url)
- else:
- issuer = getUtility(IMacaroonIssuer, 'code-import-job')
- macaroon = removeSecurityProxy(issuer).issueMacaroon(job)
- return cls(
- target_id, 'git', 'git', str(code_import.url),
- macaroon=macaroon)
- elif code_import.rcs_type == RevisionControlSystems.BZR:
- return cls(
- target_id, 'bzr', 'bzr', str(code_import.url),
- stacked_on_url=stacked_on_url)
- else:
- raise AssertionError("Unknown rcstype %r." % code_import.rcs_type)
-
- def asArguments(self):
- """Return a list of arguments suitable for passing to a child process.
- """
- result = [
- str(self.target_id), '%s:%s' % (self.rcstype, self.target_rcstype)]
- if self.rcstype in ['bzr-svn', 'git', 'bzr']:
- result.append(self.url)
- if self.stacked_on_url is not None:
- result.append(self.stacked_on_url)
- elif self.rcstype == 'cvs':
- result.append(self.cvs_root)
- result.append(self.cvs_module)
- else:
- raise AssertionError("Unknown rcstype %r." % self.rcstype)
- if self.target_rcstype == 'git':
- # XXX cjwatson 2016-10-12: Consider arranging for this to be
- # passed to worker processes in the environment instead.
- result.append(self.macaroon.serialize())
- return result
-
class ImportDataStore:
"""A store for data associated with an import.
Follow ups
