From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 4322A138247 for ; Sun, 19 Jan 2014 22:45:03 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 31C0FE0DFC; Sun, 19 Jan 2014 22:45:02 +0000 (UTC) Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 565DBE0D7C for ; Sun, 19 Jan 2014 22:45:01 +0000 (UTC) Received: by mail-wi0-f169.google.com with SMTP id e4so3844960wiv.4 for ; Sun, 19 Jan 2014 14:45:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:content-type; bh=JakK3+bYL28XXceCoDxXWiSEfLizIfKgKx4rnIDWeCs=; b=BBJUvQTwJws7Pea+g12OSPyUtS2vrBlgnHiN6tW5s3zZ7GYQEUGxXZGEy3cAWF24pX QgFhSNfcgIHsPp1YWjqCNOZRGJyf72yqZh0AlQzqc6FvA2GX6QPwrChePrz+AcOfqYMQ G2u6l2rOWC5/2sI7VVSz0OwdlENvK+2tHT0JLSjYiG8tDTQakqkoqJzr/FzLLkMBBybO QWd8dQCUOitYxAh+GvNfNFebEVwDPPjYcKR2LHtmcnFEK2AFRxaHohKKJoTRa2zz7AfL y7vZC19n+l9pn8MZxXDa36ddxn7ug4mM1IM+276kWi5jfmC9HYnwmByY5Xn1HzssFx6G gFNQ== X-Gm-Message-State: ALoCoQkOZZceHOKGBIHS/ToYmn9uzieyo2P1EOSC3WrK/i1I5V8ZakJdCf34GRcAarhhEdIO6fnW Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org MIME-Version: 1.0 X-Received: by 10.180.99.100 with SMTP id ep4mr214678wib.31.1390171499817; Sun, 19 Jan 2014 14:44:59 -0800 (PST) Sender: antarus@scriptkitty.com Received: by 10.216.170.129 with HTTP; Sun, 19 Jan 2014 14:44:59 -0800 (PST) X-Originating-IP: [173.8.165.226] In-Reply-To: References: <20140119205802.GE19935@odin.tremily.us> Date: Sun, 19 Jan 2014 14:44:59 -0800 X-Google-Sender-Auth: 2zO7HpqHPSy66ReeYKd-fBf8DyQ Message-ID: Subject: Re: [gentoo-portage-dev] [PATCH v2 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries From: Alec Warner To: gentoo-portage-dev@lists.gentoo.org Content-Type: multipart/alternative; boundary=f46d04428e5c6c852104f05a8628 X-Archives-Salt: e0f033de-21a4-4b7c-9aef-cb6a994ea51b X-Archives-Hash: 864818e466e67ea661432f7890b93f96 --f46d04428e5c6c852104f05a8628 Content-Type: text/plain; charset=UTF-8 On Sun, Jan 19, 2014 at 2:14 PM, W. Trevor King wrote: > The current fetch() function is quite long, which makes it hard to > know what I can change without adverse side effects. By pulling this > logic out of the main function, we get clearer logic in fetch() and > more explicit input for the config extraction. > --- > pym/portage/package/ebuild/fetch.py | 59 > +++++++++++++++++++++---------------- > 1 file changed, 34 insertions(+), 25 deletions(-) > > diff --git a/pym/portage/package/ebuild/fetch.py > b/pym/portage/package/ebuild/fetch.py > index 5316f03..6f46572 100644 > --- a/pym/portage/package/ebuild/fetch.py > +++ b/pym/portage/package/ebuild/fetch.py > @@ -240,6 +240,38 @@ _size_suffix_map = { > 'Y' : 80, > } > > + > +def _get_checksum_failure_max_tries(settings, default=5): > + """ > + Get the maximum number of failed download attempts > + > + Generally, downloading the same file repeatedly from > + every single available mirror is a waste of bandwidth > + and time, so there needs to be a cap. > + """ > + v = default > + try: > + v = int(settings.get("PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS", > + default)) > + except (ValueError, OverflowError): > + writemsg(_("!!! Variable > PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" > + " contains non-integer value: '%s'\n") % \ > + settings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"], > + noiselevel=-1) > + writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " > + "default value: %s\n") % default, > + noiselevel=-1) > + v = default > + if v < 1: > + writemsg(_("!!! Variable > PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" > + " contains value less than 1: '%s'\n") % v, > noiselevel=-1) > + writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " > + "default value: %s\n") % default, > + noiselevel=-1) > + v = default > + return v > + > + > This function and the next function you wrote are identical. How about making a single function? def getValueFromSettings(settings, key, default, validator): try: return validator(settings.get(key, default)) except ValidationError as e: # writemsg the exception, it will contain the juicy bits.) def getIntValueFromSettings(settings, key, default): def validator(v): try: v = int(v) except (ValueError, OverflowError) as e: raise ValidationError(" bla bla bla not an int, we picked the default.") if v < 1: v = default raise ValidationError("bla bla bla less than one we used the defualt.") return getValueFromSettings(settings, key, default, validator) Then we are not necessarily writing a bunch of the same functions over and over. The defaults we can stick in a config file, or in python, or at the call site. -A > def fetch(myuris, mysettings, listonly=0, fetchonly=0, > locks_in_subdir=".locks", use_locks=1, try_mirrors=1, digests=None, > allow_missing_digests=True): > @@ -263,31 +295,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, > print(_(">>> \"mirror\" mode desired and > \"mirror\" restriction found; skipping fetch.")) > return 1 > > - # Generally, downloading the same file repeatedly from > - # every single available mirror is a waste of bandwidth > - # and time, so there needs to be a cap. > - checksum_failure_max_tries = 5 > - v = checksum_failure_max_tries > - try: > - v = > int(mysettings.get("PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS", > - checksum_failure_max_tries)) > - except (ValueError, OverflowError): > - writemsg(_("!!! Variable > PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" > - " contains non-integer value: '%s'\n") % \ > - mysettings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"], > noiselevel=-1) > - writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " > - "default value: %s\n") % > checksum_failure_max_tries, > - noiselevel=-1) > - v = checksum_failure_max_tries > - if v < 1: > - writemsg(_("!!! Variable > PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" > - " contains value less than 1: '%s'\n") % v, > noiselevel=-1) > - writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " > - "default value: %s\n") % > checksum_failure_max_tries, > - noiselevel=-1) > - v = checksum_failure_max_tries > - checksum_failure_max_tries = v > - del v > + checksum_failure_max_tries = _get_checksum_failure_max_tries( > + settings=mysettings) > > fetch_resume_size_default = "350K" > fetch_resume_size = mysettings.get("PORTAGE_FETCH_RESUME_MIN_SIZE") > -- > 1.8.5.2.8.g0f6c0d1 > > > --f46d04428e5c6c852104f05a8628 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On S= un, Jan 19, 2014 at 2:14 PM, W. Trevor King <wking@tremily.us> wrote:
The current fetch() function is quite long, = which makes it hard to
know what I can change without adverse side effects. =C2=A0By pulling this<= br> logic out of the main function, we get clearer logic in fetch() and
more explicit input for the config extraction.
---
=C2=A0pym/portage/package/ebuild/fetch.py | 59 +++++++++++++++++++++-------= ---------
=C2=A01 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebui= ld/fetch.py
index 5316f03..6f46572 100644
--- a/pym/portage/package/ebuild/fetch.py
+++ b/pym/portage/package/ebuild/fetch.py
@@ -240,6 +240,38 @@ _size_suffix_map =3D {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 'Y' : 80,
=C2=A0}

+
+def _get_checksum_failure_max_tries(settings, default=3D5):
+ =C2=A0 =C2=A0 =C2=A0 """
+ =C2=A0 =C2=A0 =C2=A0 Get the maximum number of failed download attempts +
+ =C2=A0 =C2=A0 =C2=A0 Generally, downloading the same file repeatedly from=
+ =C2=A0 =C2=A0 =C2=A0 every single available mirror is a waste of bandwidt= h
+ =C2=A0 =C2=A0 =C2=A0 and time, so there needs to be a cap.
+ =C2=A0 =C2=A0 =C2=A0 """
+ =C2=A0 =C2=A0 =C2=A0 v =3D default
+ =C2=A0 =C2=A0 =C2=A0 try:
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 v =3D int(settings.get(&= quot;PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS",
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 default))
+ =C2=A0 =C2=A0 =C2=A0 except (ValueError, OverflowError):
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Var= iable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 " contains non-integer value: '%s'\n") % \
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 settings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"],
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 noiselevel=3D-1)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Usi= ng PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS "
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 "default value: %s\n") % default,
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 noiselevel=3D-1)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 v =3D default
+ =C2=A0 =C2=A0 =C2=A0 if v < 1:
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Var= iable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 " contains value less than 1: '%s'\n") % v, noiseleve= l=3D-1)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Usi= ng PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS "
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 "default value: %s\n") % default,
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 noiselevel=3D-1)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 v =3D default
+ =C2=A0 =C2=A0 =C2=A0 return v
+
+

This function and the next function y= ou wrote are identical. How about making a single function?

<= /div>
def getValueFromSettings(settings, key, default, validator):
=C2=A0 try:
=C2=A0 =C2=A0 return validator(settings.get(= key, default))
=C2=A0 except ValidationError as e:
=C2= =A0 =C2=A0 # writemsg the exception, it will contain the juicy bits.)
=

def getIntValueFromSettings(settings, key, default):
=C2=A0 def validator(v):
=C2=A0 =C2=A0 try:
=C2=A0= =C2=A0 =C2=A0 v =3D int(v)
=C2=A0 =C2=A0except (ValueError, Over= flowError) as e:
=C2=A0 =C2=A0 =C2=A0 raise ValidationError("= ; bla bla bla not an int, we picked the default.")
=C2=A0 =C2=A0 if v < 1:
=C2=A0 =C2=A0 =C2=A0 v =3D defaul= t
=C2=A0 =C2=A0 =C2=A0 raise ValidationError("bla bla bla le= ss than one we used the defualt.")
=C2=A0
=C2=A0 r= eturn getValueFromSettings(settings, key, default, validator)

Then we are not necessarily writing a bunch of the same= functions over and over. The defaults we can stick in a config file, or in= python, or at the call site.

-A






=
=C2=A0
=C2=A0def fetch(myuris, mysettings, listonly=3D0, fetchonly=3D0,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 locks_in_subdir=3D".locks", use_locks= =3D1, try_mirrors=3D1, digests=3DNone,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 allow_missing_digests=3DTrue):
@@ -263,31 +295,8 @@ def fetch(myuris, mysettings, listonly=3D0, fetchonly= =3D0,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 print(_(">>> \"mirror\" mode desired and= \"mirror\" restriction found; skipping fetch."))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 return 1

- =C2=A0 =C2=A0 =C2=A0 # Generally, downloading the same file repeatedly fr= om
- =C2=A0 =C2=A0 =C2=A0 # every single available mirror is a waste of bandwi= dth
- =C2=A0 =C2=A0 =C2=A0 # and time, so there needs to be a cap.
- =C2=A0 =C2=A0 =C2=A0 checksum_failure_max_tries =3D 5
- =C2=A0 =C2=A0 =C2=A0 v =3D checksum_failure_max_tries
- =C2=A0 =C2=A0 =C2=A0 try:
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 v =3D int(mysettings.get= ("PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS",
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 checksum_failure_max_tries))
- =C2=A0 =C2=A0 =C2=A0 except (ValueError, OverflowError):
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Var= iable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 " contains non-integer value: '%s'\n") % \
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 mysettings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"], noiselevel= =3D-1)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Usi= ng PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS "
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 "default value: %s\n") % checksum_failure_max_tries,
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 noiselevel=3D-1)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 v =3D checksum_failure_m= ax_tries
- =C2=A0 =C2=A0 =C2=A0 if v < 1:
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Var= iable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 " contains value less than 1: '%s'\n") % v, noiseleve= l=3D-1)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 writemsg(_("!!! Usi= ng PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS "
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 "default value: %s\n") % checksum_failure_max_tries,
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 noiselevel=3D-1)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 v =3D checksum_failure_m= ax_tries
- =C2=A0 =C2=A0 =C2=A0 checksum_failure_max_tries =3D v
- =C2=A0 =C2=A0 =C2=A0 del v
+ =C2=A0 =C2=A0 =C2=A0 checksum_failure_max_tries =3D _get_checksum_failure= _max_tries(
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 settings=3Dmysettings)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 fetch_resume_size_default =3D "350K"<= br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 fetch_resume_size =3D mysettings.get("PORT= AGE_FETCH_RESUME_MIN_SIZE")
--
1.8.5.2.8.g0f6c0d1



--f46d04428e5c6c852104f05a8628--