launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/codehosting/codeimport/worker.py'
> --- lib/lp/codehosting/codeimport/worker.py	2016-10-11 15:36:32 +0000
> +++ lib/lp/codehosting/codeimport/worker.py	2016-10-11 15:36:32 +0000
> @@ -918,3 +973,67 @@
>          """See `PullingImportWorker.probers`."""
>          from bzrlib.bzrdir import BzrProber, RemoteBzrProber
>          return [BzrProber, RemoteBzrProber]
> +
> +
> +class GitToGitImportWorker(ImportWorker):
> +    """An import worker for imports from Git to Git."""
> +
> +    def _runGit(self, *args, **kwargs):
> +        """Run git with arguments, sending output to the logger."""
> +        cmd = ["git"] + list(args)
> +        git_process = subprocess.Popen(
> +            cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, **kwargs)
> +        for line in git_process.stdout:
> +            line = line.decode("UTF-8", "replace").rstrip("\n")
> +            # Remove any user/password data from URLs.
> +            line = re.sub(r"://([^:]*:[^@]*@)(\S+)", r"://\2", line)
> +            self._logger.info(line)
> +        retcode = git_process.wait()
> +        if retcode:
> +            raise subprocess.CalledProcessError(retcode, cmd)
> +
> +    def _doImport(self):
> +        self._logger.info("Starting job.")
> +        self._logger.info(config.codeimport.git_repository_store)
> +        try:
> +            self._opener_policy.checkOneURL(self.source_details.url)

When we move these to prodstack we should probably set up some more sensible outbound rules, but I guess it'll do for now. Hopefully git can't be confused into emitting random HTTP things to stderr.

> +        except BadUrl as e:
> +            self._logger.info("Invalid URL: %s" % e)
> +            return CodeImportWorkerExitCode.FAILURE_FORBIDDEN
> +        unauth_target_url = urljoin(
> +            config.codeimport.git_repository_store,
> +            self.source_details.target_id)
> +        split = urlsplit(unauth_target_url)
> +        if split.hostname:
> +            target_netloc = ":%s@%s" % (
> +                self.source_details.macaroon.serialize(), split.hostname)
> +        else:
> +            target_netloc = ""
> +        target_url = urlunsplit([
> +            split.scheme, target_netloc, split.path, "", ""])
> +        # XXX cjwatson 2016-10-11: Ideally we'd put credentials in a
> +        # credentials store instead.  However, git only accepts credentials
> +        # that have both a non-empty username and a non-empty password.
> +        self._logger.info("Getting existing repository from hosting service.")
> +        try:
> +            self._runGit("clone", "--bare", target_url, "repository")
> +        except subprocess.CalledProcessError as e:
> +            self._logger.info(
> +                "Unable to get existing repository from hosting service: "
> +                "%s" % e)
> +            return CodeImportWorkerExitCode.FAILURE
> +        self._logger.info("Fetching remote repository.")
> +        try:
> +            self._runGit(
> +                "remote", "add", "-f", "--mirror=fetch",
> +                "mirror", self.source_details.url, cwd="repository")

HEAD handling is always atrocious... but I hadn't realised git-remote fiddled around so manually in ways that aren't exposed elsewhere.

> +        except subprocess.CalledProcessError as e:
> +            self._logger.info("Unable to fetch remote repository: %s" % e)
> +            return CodeImportWorkerExitCode.FAILURE_INVALID
> +        self._logger.info("Pushing repository to hosting service.")
> +        try:
> +            self._runGit("push", "--mirror", target_url, cwd="repository")
> +        except subprocess.CalledProcessError as e:
> +            self._logger.info("Unable to push to hosting service: %s" % e)
> +            return CodeImportWorkerExitCode.FAILURE
> +        return CodeImportWorkerExitCode.SUCCESS
> 
> === modified file 'lib/lp/services/config/schema-lazr.conf'
> --- lib/lp/services/config/schema-lazr.conf	2016-10-11 15:36:32 +0000
> +++ lib/lp/services/config/schema-lazr.conf	2016-10-11 15:36:32 +0000
> @@ -417,7 +421,7 @@
>  svn_revisions_import_limit: 500
>  
>  # Secret key for macaroons used to grant git push permission to workers.
> -macaroon_secret_key:
> +macaroon_secret_key: none

I have no words, but carry on.

>  
>  [codeimportdispatcher]
>  # The directory where the code import worker should be directed to


-- 
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-worker/+merge/308142
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.