[Merge] lp:~vorlon/ddeb-retriever/python3 into lp:ddeb-retriever

[Colin Watson]

Review: Approve



Diff comments:

> === modified file 'archive_tools.py'
> --- archive_tools.py	2017-04-22 23:07:59 +0000
> +++ archive_tools.py	2017-05-23 20:01:39 +0000
> @@ -158,7 +157,7 @@
>          if cmp(max.get(parser.section[group_by], ('', ''))[0], parser.section[sort_key]) < 0:
>              max[parser.section[group_by]] = (parser.section[sort_key], str(parser.section))
>  
> -    ks = max.keys()
> +    ks = list(max.keys())
>      ks.sort()

ks = sorted(max)

>      first = True
>      for k in ks:
> @@ -534,7 +540,7 @@
>              d = os.path.join(self.workdir, repo, 'dists', pocket, component, 'binary-' + arch)
>              if not os.path.isdir(d):
>                  os.makedirs(d)
> -            f = gzip.GzipFile(os.path.join(d, 'Packages.gz'), mode='w')
> +            f = gzip.open(os.path.join(d, 'Packages.gz'), mode='wt')
>              for (name, version) in packages:
>                  f.write('Package: %s\nVersion: %s\n\n' % (name, version))
>              f.close()

with gzip.open(...) as f:
    ...

> @@ -710,7 +716,7 @@
>      '''Return string with space-separated sorted list of keys of a
>      dictionary.'''
>  
> -    k = map.keys()
> +    k = list(map.keys())
>      k.sort()

k = sorted(map)

>      return ' '.join(k)
>  
> 
> === modified file 'ddeb_retriever.py'
> --- ddeb_retriever.py	2017-04-22 23:07:59 +0000
> +++ ddeb_retriever.py	2017-05-23 20:01:39 +0000
> @@ -107,7 +107,14 @@
>              os.makedirs(destdir)
>          except OSError:
>              pass
> -        urllib.urlretrieve(url, dest)
> +        with urllib.request.urlopen(url) as req:
> +            if req.getcode() != 200:
> +                logging.error('URL %s failed with code %u', req.geturl(), code)
> +                return False
> +            response = req.read()
> +        with open(dest, 'wb') as handle:
> +            handle.write(response)

Since the destination here is in the output tree, I think this should also write the response content to a temporary file (on the same file system) and then rename it into place; otherwise we could still end up with truncated files if ddeb-retriever is interrupted.

> +
>          logging.debug('Downloaded %s into %s', ddeb, os.path.dirname(dest))
>  
>      return True


-- 
https://code.launchpad.net/~vorlon/ddeb-retriever/python3/+merge/324233
Your team Ubuntu Package Archive Administrators is subscribed to branch lp:ddeb-retriever.