1 |
On Sun, Jan 19, 2014 at 2:44 PM, Alec Warner <antarus@g.o> wrote: |
2 |
|
3 |
> On Sun, Jan 19, 2014 at 2:14 PM, W. Trevor King <wking@×××××××.us> wrote: |
4 |
> |
5 |
>> The current fetch() function is quite long, which makes it hard to |
6 |
>> know what I can change without adverse side effects. By pulling this |
7 |
>> logic out of the main function, we get clearer logic in fetch() and |
8 |
>> more explicit input for the config extraction. |
9 |
>> --- |
10 |
>> pym/portage/package/ebuild/fetch.py | 59 |
11 |
>> +++++++++++++++++++++---------------- |
12 |
>> 1 file changed, 34 insertions(+), 25 deletions(-) |
13 |
>> |
14 |
>> diff --git a/pym/portage/package/ebuild/fetch.py |
15 |
>> b/pym/portage/package/ebuild/fetch.py |
16 |
>> index 5316f03..6f46572 100644 |
17 |
>> --- a/pym/portage/package/ebuild/fetch.py |
18 |
>> +++ b/pym/portage/package/ebuild/fetch.py |
19 |
>> @@ -240,6 +240,38 @@ _size_suffix_map = { |
20 |
>> 'Y' : 80, |
21 |
>> } |
22 |
>> |
23 |
>> + |
24 |
>> +def _get_checksum_failure_max_tries(settings, default=5): |
25 |
>> + """ |
26 |
>> + Get the maximum number of failed download attempts |
27 |
>> + |
28 |
>> + Generally, downloading the same file repeatedly from |
29 |
>> + every single available mirror is a waste of bandwidth |
30 |
>> + and time, so there needs to be a cap. |
31 |
>> + """ |
32 |
>> + v = default |
33 |
>> + try: |
34 |
>> + v = int(settings.get("PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS", |
35 |
>> + default)) |
36 |
>> + except (ValueError, OverflowError): |
37 |
>> + writemsg(_("!!! Variable |
38 |
>> PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" |
39 |
>> + " contains non-integer value: '%s'\n") % \ |
40 |
>> + settings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"], |
41 |
>> + noiselevel=-1) |
42 |
>> + writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " |
43 |
>> + "default value: %s\n") % default, |
44 |
>> + noiselevel=-1) |
45 |
>> + v = default |
46 |
>> + if v < 1: |
47 |
>> + writemsg(_("!!! Variable |
48 |
>> PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" |
49 |
>> + " contains value less than 1: '%s'\n") % v, |
50 |
>> noiselevel=-1) |
51 |
>> + writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " |
52 |
>> + "default value: %s\n") % default, |
53 |
>> + noiselevel=-1) |
54 |
>> + v = default |
55 |
>> + return v |
56 |
>> + |
57 |
>> + |
58 |
>> |
59 |
> |
60 |
> This function and the next function you wrote are identical. How about |
61 |
> making a single function? |
62 |
> |
63 |
> def getValueFromSettings(settings, key, default, validator): |
64 |
> try: |
65 |
> return validator(settings.get(key, default)) |
66 |
> except ValidationError as e: |
67 |
> # writemsg the exception, it will contain the juicy bits.) |
68 |
> |
69 |
> def getIntValueFromSettings(settings, key, default): |
70 |
> def validator(v): |
71 |
> try: |
72 |
> v = int(v) |
73 |
> except (ValueError, OverflowError) as e: |
74 |
> raise ValidationError(" bla bla bla not an int, we picked the |
75 |
> default.") |
76 |
> if v < 1: |
77 |
> v = default |
78 |
> raise ValidationError("bla bla bla less than one we used the |
79 |
> defualt.") |
80 |
> |
81 |
> return getValueFromSettings(settings, key, default, validator) |
82 |
> |
83 |
> |
84 |
Note, don't actually use these function names, they are terrible. |
85 |
|
86 |
-A |
87 |
|
88 |
|
89 |
|
90 |
> Then we are not necessarily writing a bunch of the same functions over and |
91 |
> over. The defaults we can stick in a config file, or in python, or at the |
92 |
> call site. |
93 |
> |
94 |
> -A |
95 |
> |
96 |
> |
97 |
> |
98 |
> |
99 |
> |
100 |
> |
101 |
> |
102 |
> |
103 |
>> def fetch(myuris, mysettings, listonly=0, fetchonly=0, |
104 |
>> locks_in_subdir=".locks", use_locks=1, try_mirrors=1, |
105 |
>> digests=None, |
106 |
>> allow_missing_digests=True): |
107 |
>> @@ -263,31 +295,8 @@ def fetch(myuris, mysettings, listonly=0, |
108 |
>> fetchonly=0, |
109 |
>> print(_(">>> \"mirror\" mode desired and |
110 |
>> \"mirror\" restriction found; skipping fetch.")) |
111 |
>> return 1 |
112 |
>> |
113 |
>> - # Generally, downloading the same file repeatedly from |
114 |
>> - # every single available mirror is a waste of bandwidth |
115 |
>> - # and time, so there needs to be a cap. |
116 |
>> - checksum_failure_max_tries = 5 |
117 |
>> - v = checksum_failure_max_tries |
118 |
>> - try: |
119 |
>> - v = |
120 |
>> int(mysettings.get("PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS", |
121 |
>> - checksum_failure_max_tries)) |
122 |
>> - except (ValueError, OverflowError): |
123 |
>> - writemsg(_("!!! Variable |
124 |
>> PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" |
125 |
>> - " contains non-integer value: '%s'\n") % \ |
126 |
>> - mysettings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"], |
127 |
>> noiselevel=-1) |
128 |
>> - writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " |
129 |
>> - "default value: %s\n") % |
130 |
>> checksum_failure_max_tries, |
131 |
>> - noiselevel=-1) |
132 |
>> - v = checksum_failure_max_tries |
133 |
>> - if v < 1: |
134 |
>> - writemsg(_("!!! Variable |
135 |
>> PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS" |
136 |
>> - " contains value less than 1: '%s'\n") % v, |
137 |
>> noiselevel=-1) |
138 |
>> - writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS " |
139 |
>> - "default value: %s\n") % |
140 |
>> checksum_failure_max_tries, |
141 |
>> - noiselevel=-1) |
142 |
>> - v = checksum_failure_max_tries |
143 |
>> - checksum_failure_max_tries = v |
144 |
>> - del v |
145 |
>> + checksum_failure_max_tries = _get_checksum_failure_max_tries( |
146 |
>> + settings=mysettings) |
147 |
>> |
148 |
>> fetch_resume_size_default = "350K" |
149 |
>> fetch_resume_size = |
150 |
>> mysettings.get("PORTAGE_FETCH_RESUME_MIN_SIZE") |
151 |
>> -- |
152 |
>> 1.8.5.2.8.g0f6c0d1 |
153 |
>> |
154 |
>> |
155 |
>> |
156 |
> |