Re: [gentoo-portage-dev] [PATCH v2 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries

[Alec Warner <antarus@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 5191 bytes --]

On Sun, 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.  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
>
>
>

[-- Attachment #2: Type: text/html, Size: 7069 bytes --]