launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> === modified file 'lib/lp/code/githosting.py'
> --- lib/lp/code/githosting.py	2015-03-20 11:52:54 +0000
> +++ lib/lp/code/githosting.py	2015-04-01 21:12:48 +0000
> @@ -37,15 +37,19 @@
>          # over, but is there some more robust way to do this?
>          return 5.0
>  
> -    def create(self, path):
> +    def create(self, path, clone_from=None):
>          try:
>              # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
>              # should just use post(json=) and drop the explicit Content-Type
>              # header.
> +            if clone_from:
> +                request = {"repo_path": path, "clone_from": clone_from}
> +            else:
> +                request = {"repo_path": path}
>              response = self._makeSession().post(
>                  urlutils.join(self.endpoint, "repo"),
>                  headers={"Content-Type": "application/json"},
> -                data=json.dumps({"repo_path": path, "bare_repo": True}),
> +                data=json.dumps(request),
>                  timeout=self.timeout)
>          except Exception as e:
>              raise GitRepositoryCreationFault(
> 
> === modified file 'lib/lp/code/xmlrpc/git.py'
> --- lib/lp/code/xmlrpc/git.py	2015-03-31 16:29:51 +0000
> +++ lib/lp/code/xmlrpc/git.py	2015-04-01 21:12:48 +0000
> @@ -24,6 +24,7 @@
>      GitRepositoryCreationForbidden,
>      GitRepositoryCreationFault,
>      GitRepositoryExists,
> +    GitTargetError,
>      InvalidNamespace,
>      )
>  from lp.code.githosting import GitHostingClient
> @@ -67,6 +68,7 @@
>          super(GitAPI, self).__init__(*args, **kwargs)
>          self.hosting_client = GitHostingClient(
>              config.codehosting.internal_git_api_endpoint)
> +        self.repository_set = getUtility(IGitRepositorySet)
>  
>      def _performLookup(self, path):
>          repository, extra_path = getUtility(IGitLookup).getByPath(path)
> @@ -122,12 +124,11 @@
>                  "target; push to a named repository instead.")
>          if repository_name is None:
>              def default_func(new_repository):
> -                repository_set = getUtility(IGitRepositorySet)
>                  if owner is None:
> -                    repository_set.setDefaultRepository(
> +                    self.repository_set.setDefaultRepository(
>                          target, new_repository)
>                  else:
> -                    repository_set.setDefaultRepositoryForOwner(
> +                    self.repository_set.setDefaultRepositoryForOwner(
>                          owner, target, new_repository)
>  
>              repository_name = namespace.findUnusedName(target.name)
> @@ -146,7 +147,7 @@
>          getUtility(IErrorReportingUtility).raising(sys.exc_info(), request)
>          raise faults.OopsOccurred("creating a Git repository", request.oopsid)
>  
> -    def _createRepository(self, requester, path):
> +    def _createRepository(self, requester, path, clone_from=None):
>          try:
>              namespace, repository_name, default_func = (
>                  self._getGitNamespaceExtras(path, requester))
> @@ -196,9 +197,19 @@
>              Store.of(repository).flush()
>              assert repository.id is not None
>  
> +            # If repository has target_default, clone from default.
> +            target_path = None
> +            try:
> +                target_default = self.repository_set.getDefaultRepository(
> +                    repository.target)
> +                if target_default and target_default.visibleByUser(requester):
> +                    target_path = target_default.getInternalPath()
> +            except GitTargetError:
> +                pass # Ignore Personal repositories.

This should additionally fall back to an owner-target default if there isn't a target default.

> +
>              hosting_path = repository.getInternalPath()
>              try:
> -                self.hosting_client.create(hosting_path)
> +                self.hosting_client.create(hosting_path, clone_from=target_path)
>              except GitRepositoryCreationFault as e:
>                  # The hosting service failed.  Log an OOPS for investigation.
>                  self._reportError(path, e, hosting_path=hosting_path)
> 
> === modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
> --- lib/lp/code/xmlrpc/tests/test_git.py	2015-03-31 04:29:24 +0000
> +++ lib/lp/code/xmlrpc/tests/test_git.py	2015-04-01 21:12:48 +0000
> @@ -43,14 +43,14 @@
>      def __init__(self):
>          self.calls = []
>  
> -    def create(self, path):
> -        self.calls.append(("create", path))
> +    def create(self, path, clone_from=None):
> +        self.calls.append(("create", path, clone_from))
>  
>  
>  class BrokenGitHostingClient:
>      """A GitHostingClient lookalike that pretends the remote end is down."""
>  
> -    def create(self, path):
> +    def create(self, path, clone_from=None):
>          raise GitRepositoryCreationFault("nothing here")
>  
>  
> @@ -109,6 +109,7 @@
>          self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
>          self.git_api = GitAPI(None, None)
>          self.git_api.hosting_client = FakeGitHostingClient()
> +        self.repository_set = getUtility(IGitRepositorySet)
>  
>      def assertPathTranslationError(self, requester, path, permission="read",
>                                     can_authenticate=False):
> @@ -212,10 +213,18 @@
>               "trailing": ""},
>              translation)
>          self.assertEqual(
> -            [("create", repository.getInternalPath())],
> -            self.git_api.hosting_client.calls)
> +            ("create", repository.getInternalPath()),
> +            self.git_api.hosting_client.calls[0][0:2])
>          return repository
>  
> +    def assertCreatesFromClone(self, requester, path, can_authenticate=False):
> +        repository = self.assertCreates(requester, path, can_authenticate)
> +        target_default = removeSecurityProxy(
> +            self.repository_set.getDefaultRepository(repository.target))
> +        self.assertIsNotNone(target_default)
> +        self.assertEqual(target_default.getInternalPath(),
> +                         self.git_api.hosting_client.calls[0][2])
> +
>      def test_translatePath_private_repository(self):
>          requester = self.factory.makePerson()
>          repository = removeSecurityProxy(
> @@ -418,6 +427,18 @@
>          self.assertCreates(
>              requester, u"/~%s/%s/+git/random" % (requester.name, project.name))
>  
> +    def test_translatePath_create_project_with_default_target(self):
> +        # translatePath creates a project repository cloned from the
> +        # target default if it exists.
> +        target = self.factory.makeProduct()
> +        repository = self.factory.makeGitRepository(
> +            owner=target.owner, target=target)
> +        with person_logged_in(target.owner):
> +            self.repository_set.setDefaultRepository(target, repository)
> +            self.assertCreatesFromClone(
> +                target.owner, u"/~%s/%s/+git/random" % (target.owner.name,
> +                                                        target.name))
> +
>      def test_translatePath_create_package(self):
>          # translatePath creates a package repository that doesn't exist, if
>          # it can.
> 


-- 
https://code.launchpad.net/~blr/launchpad/init-with-clone/+merge/254702
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.