[Simple-cdd-devel] Bug#861198: Shutting down public FTP services

[Vagrant Cascadian]

On 2017-05-15, Enrico Zini wrote:
> On Sun, May 07, 2017 at 09:58:12PM -0700, Vagrant Cascadian wrote:
>
>> Any chance you could review the urllib implementation that is in git
>> soonish? :)
>
> I've been adventurously busy, I hope it's not too late.

Thanks for the review! :)


> In bfc0329060e262bfd889b5e40c32ad9099197dcb I have mixed feelings about
> this, as it uses string split on a path while python has os.path.split.
> However, I couldn't at a glance follow the code to see what kind of
> input it works on, and if it's something like
> http://ftp.fr.debian.org/debian/dists/jessie/Release
> then the lines are not really paths:
>
> -                    prefix_len = 7 + len(env.get("DI_CODENAME")) # dists/{DI_CODENAME}/
> -                    relname = file[prefix_len:]
> +                    separator = os.path.join('dists/', env.get("DI_CODENAME"), '')
> +                    separator, relname = file.split(separator)

If I remember correctly, it is trying to split a filename based on
dists/CODENAME, so maybe os.path.split would be appropriate here; will
look into it.


> In b164d54b912a8d702576c191a821ec2f374642cf I agree that except without
> an exception is too broad, like in here:
>
>                     try:
>                         checksums.verify_file(output, relname)
>                     except:
>                         raise Fail("Checksum invalid: %s", output)
>
> I noticed that verify_file in simple_cdd/utils.py already throws Fail if
> something is wrong. I would get rid of the try/except block altogether,
> and just call verify_file.

I think with these I was getting an ugly traceback when the file failed
to verify, so wanted to actually give the user something more freindly
than a traceback... will try your patch again, tweak the file between
download and verify, and see if it still spits out the tracebacks.


live well,
  vagrant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/simple-cdd-devel/attachments/20170515/138dc66d/attachment.sig>