On Sun, Jan 19, 2014 at 2:44 PM, Alec Warner <antarus@gentoo.org> wrote:

> 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)
>
>
Note, don't actually use these function names, they are terrible.

-A



> 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
>>
>>
>>
>