Re: [gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
On Sat, 18 Jan 2014 19:07:46 -0800 W. Trevor King wk...@tremily.us wrote: +def _get_fetch_resume_size(settings, default='350K'): + v = settings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) + if v is not None: + v = .join(v.split()) + if not v: + # If it's empty, silently use the default. + v = default + match = _fetch_resume_size_re.match(v) + if (match is None or + match.group(2).upper() not in _size_suffix_map): + writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE + contains an unrecognized format: '%s'\n) % \ + settings[PORTAGE_FETCH_RESUME_MIN_SIZE], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE + default value: %s\n) % default, + noiselevel=-1) + v = None + if v is None: + v = default + match = _fetch_resume_size_re.match(v) + v = int(match.group(1)) * \ + 2 ** _size_suffix_map[match.group(2).upper()] + return v There is some duplicate code here, I think the conditions can be rewritten in such way that the duplicate code doesn't take place. Move everything from the first 'if' except for the first line out of that 'if', that way get a bare: if v is not None: v = .join(v.split()) Outside that, you now have 'if not v:' whereas you later have 'if v is None:'; you can optimize this to 'if not v or v is None:' (maybe that can be optimized further, dunno) and just have one condition set the default here. This gives you: if not v or v is None: # If it's empty, silently use the default. v = default Afterwards, you have 'match = _fetch_resume_size_re.match(v)' in both places, you can again just have this once. Can the two remaining code blocks just be placed after that? -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
On Mon, Jan 20, 2014 at 02:41:41AM +0100, Tom Wijsman wrote: There is some duplicate code here, I think the conditions can be rewritten in such way that the duplicate code doesn't take place. Do you want a rewrite squashed into this commit, or as a follow-on commit after this one (which gets a test suite in v3)? Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
On Sun, 19 Jan 2014 18:01:23 -0800 W. Trevor King wk...@tremily.us wrote: On Mon, Jan 20, 2014 at 02:41:41AM +0100, Tom Wijsman wrote: There is some duplicate code here, I think the conditions can be rewritten in such way that the duplicate code doesn't take place. Do you want a rewrite squashed into this commit, or as a follow-on commit after this one (which gets a test suite in v3)? Sound more sane to do in a follow-up commit. While writing this review I didn't note that you were just moving most code, now you have ideas for further refactoring I guess. :) -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
[gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
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 | 50 - 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 4337247..bd572fa 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -271,6 +271,32 @@ def _get_checksum_failure_max_tries(settings, default=5): return v +def _get_fetch_resume_size(settings, default='350K'): + v = settings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) + if v is not None: + v = .join(v.split()) + if not v: + # If it's empty, silently use the default. + v = default + match = _fetch_resume_size_re.match(v) + if (match is None or + match.group(2).upper() not in _size_suffix_map): + writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE +contains an unrecognized format: '%s'\n) % \ + settings[PORTAGE_FETCH_RESUME_MIN_SIZE], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE + default value: %s\n) % default, + noiselevel=-1) + v = None + if v is None: + v = default + match = _fetch_resume_size_re.match(v) + v = int(match.group(1)) * \ + 2 ** _size_suffix_map[match.group(2).upper()] + return v + + def fetch(myuris, mysettings, listonly=0, fetchonly=0, locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None, allow_missing_digests=True): @@ -296,29 +322,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, 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) - if fetch_resume_size is not None: - fetch_resume_size = .join(fetch_resume_size.split()) - if not fetch_resume_size: - # If it's undefined or empty, silently use the default. - fetch_resume_size = fetch_resume_size_default - match = _fetch_resume_size_re.match(fetch_resume_size) - if match is None or \ - (match.group(2).upper() not in _size_suffix_map): - writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE -contains an unrecognized format: '%s'\n) % \ - mysettings[PORTAGE_FETCH_RESUME_MIN_SIZE], noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE - default value: %s\n) % fetch_resume_size_default, - noiselevel=-1) - fetch_resume_size = None - if fetch_resume_size is None: - fetch_resume_size = fetch_resume_size_default - match = _fetch_resume_size_re.match(fetch_resume_size) - fetch_resume_size = int(match.group(1)) * \ - 2 ** _size_suffix_map[match.group(2).upper()] + fetch_resume_size = _get_fetch_resume_size(settings=mysettings) # Behave like the package has RESTRICT=primaryuri after a # couple of checksum failures, to increase the probablility -- 1.8.5.2.8.g0f6c0d1
