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. 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) About 6717a008549cd792a4c08d7c21d09cbd8b82a67a: COOL! 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. 1faddb9cfe4992c36bca7e502f5190e96de74f77 looks good, I can't think of a better way of doing it. I'm attaching a proposed patch for the over-broad excepts. Enrico -- GPG key: 4096R/634F4BD1E7AD5568 2009-05-08 Enrico Zini <[email protected]>
From c84da64e7485cc89d0c72ab5d15c3df9f58945b5 Mon Sep 17 00:00:00 2001 From: Enrico Zini <[email protected]> Date: Mon, 15 May 2017 12:41:33 +0200 Subject: [PATCH] Avoid except: that might catch too much --- simple_cdd/tools/mirror_wget.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/simple_cdd/tools/mirror_wget.py b/simple_cdd/tools/mirror_wget.py index 73999a3..dc914cf 100644 --- a/simple_cdd/tools/mirror_wget.py +++ b/simple_cdd/tools/mirror_wget.py @@ -46,7 +46,7 @@ class ToolMirrorWget(Tool): checksums.verify_file(output, relname) log.debug("skipping download: %s checksum matched", output) return - except: + except Fail: log.debug("re-downloading: %s checksum invalid", output) pass if not os.path.isdir(os.path.dirname(output)): @@ -54,10 +54,7 @@ class ToolMirrorWget(Tool): log.debug("downloading: %s", output) request.urlretrieve(url, filename=output) if checksums: - try: - checksums.verify_file(output, relname) - except: - raise Fail("Checksum invalid: %s", output) + checksums.verify_file(output, relname) if env.get("mirror_files"): # Download the checksums present in the archive "extrafiles" and verify -- 2.11.0
signature.asc
Description: PGP signature
