launchpad-reviewers team mailing list archive

[Bayard 'kit' Randel <kit.randel@xxxxxxxxxxxxx>]

Thanks Colin, I'll catch up with Martin and make those changes.

On Mon, Oct 5, 2015 at 11:50 AM Colin Watson <cjwatson@xxxxxxxxxxxxx> wrote:

> Review: Approve
>
> It sounds like the direction we're going is indeed going to be to provide
> external access for the whole duration of the build, but could you please
> get in touch with Martin Albisetti to confirm that that's definitely what
> we have to do rather than separating out the pull phase and providing
> external access only to that as we'd previously intended?
>
> Diff comments:
>
> > === modified file 'lpbuildd/snap.py'
> > --- lpbuildd/snap.py  2015-07-31 11:54:07 +0000
> > +++ lpbuildd/snap.py  2015-10-04 22:12:07 +0000
> > @@ -52,6 +56,15 @@
> >              "--build-id", self._buildid,
> >              "--arch", self.arch_tag,
> >              ]
> > +        if self.proxy_token:
>
> Maybe worth checking here that all the necessary arguments were passed,
> just in case.
>
> > +            proxy_env = (
> > +                "http_proxy=http://{username}:{password}";
> > +                "@{host}:{port}".format(
> > +                    username=self.build_cookie,
> > +                    password=self.proxy_token,
> > +                    host=self.proxy_host,
> > +                    port=self.proxy_port))
> > +            args.insert(0, proxy_env)
>
> Rather than relying on shell parsing of environment variable prefixes to
> the command, could you please instead add an env=None keyword argument to
> runSubProcess, defaulting to os.environ if None, so that you can then do
> something like:
>
>   env = dict(os.environ)
>   if condition:
>       env["http_proxy"] = blah
>   ...
>   self.runSubProcess(self.build_snap_path, args, env=env)
>
> >          if self.branch is not None:
> >              args.extend(["--branch", self.branch])
> >          if self.git_repository is not None:
>
>
> --
>
> https://code.launchpad.net/~blr/launchpad-buildd/snap_http_proxy/+merge/273355
> You are the owner of lp:~blr/launchpad-buildd/snap_http_proxy.
>

-- 
https://code.launchpad.net/~blr/launchpad-buildd/snap_http_proxy/+merge/273355
Your team Launchpad code reviewers is subscribed to branch lp:launchpad-buildd.