launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/bugs/scripts/cveimport.py'
> --- lib/lp/bugs/scripts/cveimport.py	2016-08-16 15:33:02 +0000
> +++ lib/lp/bugs/scripts/cveimport.py	2018-06-02 21:05:54 +0000
> @@ -209,14 +213,22 @@
>          elif self.options.cveurl is not None:
>              self.logger.info("Downloading CVE database from %s..." %
>                               self.options.cveurl)
> +            proxies = {}
> +            if config.launchpad.http_proxy:
> +                proxies['http'] = config.launchpad.http_proxy
> +                proxies['https'] = config.launchpad.http_proxy

I wonder if this should be inlined into urlfetch(use_proxy=True) at this point?

>              try:
> -                url = urllib2.urlopen(self.options.cveurl)
> -            except (urllib2.HTTPError, urllib2.URLError):
> +                with override_timeout(config.cveupdater.timeout):
> +                    # Command-line options are trusted, so allow file://
> +                    # URLs to ease testing.
> +                    response = urlfetch(
> +                        self.options.cveurl, proxies=proxies, allow_file=True)
> +            except requests.RequestException:
>                  raise LaunchpadScriptFailure(
>                      'Unable to connect for CVE database %s'
>                      % self.options.cveurl)
>  
> -            cve_db_gz = url.read()
> +            cve_db_gz = response.content
>              self.logger.info("%d bytes downloaded." % len(cve_db_gz))
>              cve_db = gzip.GzipFile(
>                  fileobj=StringIO.StringIO(cve_db_gz)).read()


-- 
https://code.launchpad.net/~cjwatson/launchpad/cveimport-requests/+merge/347201
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.