Thanks Colin, I'll catch up with Martin and make those changes. On Mon, Oct 5, 2015 at 11:50 AM Colin Watson <[email protected]> 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. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
