Merge lp:~jml/pkgme/juggle-backend-selector into lp:pkgme

Proposed by Jonathan Lange
Status: Merged
Approved by: James Westby
Approved revision: 148
Merged at revision: 137
Proposed branch: lp:~jml/pkgme/juggle-backend-selector
Merge into: lp:pkgme
Diff against target: 269 lines (+87/-56)
5 files modified
NEWS.txt (+11/-0)
pkgme/backend.py (+55/-48)
pkgme/bin/main.py (+8/-3)
pkgme/tests/test_backend.py (+9/-4)
pkgme/tests/test_script.py (+4/-1)
To merge this branch: bzr merge lp:~jml/pkgme/juggle-backend-selector
Reviewer Review Type Date Requested Status
James Westby Approve
Review via email: mp+122225@code.launchpad.net

Commit message

Make BackendSelector more re-usable

Description of the change

We want to expose more data about what pkgme is doing to Python API callers.

This branch begins that by factoring out a lot of the guts of
``BackendSelector.get_info``, a very important method in determining what
pkgme does.

I intend to follow it up with other branches that eventually present a
consistent layer for both the command line script and things like pkgme-service
to use.

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

Looks great, thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS.txt'
2--- NEWS.txt 2012-08-28 10:57:30 +0000
3+++ NEWS.txt 2012-08-31 10:10:23 +0000
4@@ -3,6 +3,17 @@
5 NEWS for pkgme
6 ==============
7
8+
9+NEXT
10+====
11+
12+Changes
13+-------
14+
15+ * ``BackendSelector.get_eligible_backends`` returns a list of dicts, rather
16+ than a list of ``(score, backend)`` 2-tuples. (Jonathan Lange)
17+
18+
19 0.3.1 (2012-08-28)
20 ==================
21
22
23=== modified file 'pkgme/backend.py'
24--- pkgme/backend.py 2012-08-24 13:48:10 +0000
25+++ pkgme/backend.py 2012-08-31 10:10:23 +0000
26@@ -1,4 +1,5 @@
27 import json
28+from operator import itemgetter
29 import os
30 import sys
31
32@@ -265,23 +266,60 @@
33 class NoEligibleBackend(PkgmeError):
34 """Raised when pkgme cannot find an appropriate backend."""
35
36- def __init__(self, path, tried_backends,
37- disallowed_backends=None):
38- backend_names = []
39- for backend, reason in tried_backends:
40- if reason:
41- name = '%s (%s)' % (backend.name, reason)
42- else:
43- name = backend.name
44- backend_names.append(name)
45- backend_names = sorted(backend_names)
46+ def __init__(self, path, tried_backends, disallowed_backends=None):
47+ backend_names = sorted(map(self._describe_result, tried_backends))
48 msg = ("No eligible backends for %s. Tried %s"
49 % (path, ', '.join(backend_names)))
50- if disallowed_backends is not None:
51+ if disallowed_backends:
52 msg += (". The following backends were disallowed by policy: %s."
53- % (', '.join(disallowed_backends)))
54+ % (', '.join([b.name for b in disallowed_backends])))
55 super(NoEligibleBackend, self).__init__(msg)
56
57+ @staticmethod
58+ def _describe_result(result):
59+ name = result['backend'].name
60+ reason = result.get('reason', None)
61+ if reason:
62+ name = '%s (%s)' % (name, reason)
63+ return name
64+
65+
66+def dichotomy(predicate, collection):
67+ false, true = [], []
68+ for x in collection:
69+ if predicate(x):
70+ true.append(x)
71+ else:
72+ false.append(x)
73+ return false, true
74+
75+
76+def _query_backend(path, backend):
77+ score, reason = backend.want(path)
78+ return {'score': score, 'backend': backend, 'reason': reason}
79+
80+
81+def _query_backends(path, backends):
82+ return (_query_backend(path, backend) for backend in backends)
83+
84+
85+def _is_eligible(backend):
86+ return backend['score'] > 0
87+
88+
89+def query_backends(path, backends, allowed_backend_names=None):
90+ if not backends:
91+ raise NoBackend()
92+ if allowed_backend_names is None:
93+ disallowed, allowed = [], backends
94+ else:
95+ disallowed, allowed = dichotomy(
96+ lambda b: b.name in allowed_backend_names, backends)
97+ results = _query_backends(path, allowed)
98+ ineligible, eligible = dichotomy(_is_eligible, results)
99+ eligible.sort(key=itemgetter('score'), reverse=True)
100+ return disallowed, ineligible, eligible
101+
102
103 class BackendSelector(object):
104
105@@ -298,24 +336,6 @@
106 self.backends = backends
107 self.allowed_backend_names = allowed_backend_names
108
109- def _split_disallowed_backends(self, backends):
110- """Split backends into those allowed and those not.
111-
112- :return: (disallowed, allowed), where ``disallowed`` is a list
113- of disallowed backend names and ``allowed`` is a list of
114- allowed backends. If all backends are allowed then ``disallowed``
115- is None.
116- """
117- if self.allowed_backend_names is None:
118- return None, list(backends)
119- disallowed, allowed = [], []
120- for backend in backends:
121- if backend.name in self.allowed_backend_names:
122- allowed.append(backend)
123- else:
124- disallowed.append(backend.name)
125- return disallowed, allowed
126-
127 def get_eligible_backends(self, path):
128 """Get the backends that could possibly work for ``path``.
129
130@@ -324,30 +344,17 @@
131 :return: ``[(score, backend), ...]``. Will always be sorted with
132 the highest score first, and then lexographically by backend name.
133 """
134- if not self.backends:
135- raise NoBackend()
136- disallowed, backends = self._split_disallowed_backends(self.backends)
137- eligible = []
138- ineligible = []
139- for backend in backends:
140- score, reason = backend.want(path)
141- trace.debug(' %s scored %s' % (backend.name, score))
142- if score > 0:
143- eligible.append((score, backend))
144- else:
145- ineligible.append((backend, reason))
146+ disallowed, ineligible, eligible = query_backends(
147+ path, self.backends, self.allowed_backend_names)
148 if not eligible:
149- # FIXME: some info about the problem. Bug 809423.
150 raise NoEligibleBackend(
151- path, ineligible,
152- disallowed_backends=disallowed)
153- return sorted(
154- eligible, key=lambda (score, backend): (-score, backend.name))
155+ path, ineligible, disallowed_backends=disallowed)
156+ return eligible
157
158 def get_info(self, path):
159 trace.debug('Finding backend for %s' % (path,))
160 eligble = self.get_eligible_backends(path)
161- backend = eligble[0][1]
162+ backend = eligble[0]['backend']
163 if len(eligble) > 1:
164 trace.debug(
165 '%s eligble backends. Picked %s' % (len(eligble), backend.name))
166
167=== modified file 'pkgme/bin/main.py'
168--- pkgme/bin/main.py 2012-07-20 19:24:04 +0000
169+++ pkgme/bin/main.py 2012-08-31 10:10:23 +0000
170@@ -81,7 +81,12 @@
171 """Return a string listing eligible backends for ``target_dir``."""
172 selector = get_backend_selector()
173 backends = selector.get_eligible_backends(target_dir)
174- return ', '.join('%s (%s)' % (backend.name, score) for (score, backend) in backends)
175+ return ', '.join('%s (%s)' % (r['backend'].name, r['score']) for r in backends)
176+
177+
178+def _encode_backend(result):
179+ result.update({'backend': result['backend'].name})
180+ return result
181
182
183 def get_all_info(target_dir, selector=None):
184@@ -96,7 +101,7 @@
185 if selector is None:
186 selector = get_backend_selector()
187 eligible = selector.get_eligible_backends(target_dir)
188- backend = eligible[0][1]
189+ backend = eligible[0]['backend']
190 # XXX: This ignores ExtraFiles and ExtraFilesFromPaths because they aren't
191 # in get_elements().
192 elements = default_package_file_group.get_elements()
193@@ -104,7 +109,7 @@
194 project_info = backend.get_info(target_dir)
195 data = project_info.get_all(keys)
196 data[u'selected_backend'] = backend.name
197- data[u'eligible_backends'] = [(score, b.name) for (score, b) in eligible]
198+ data[u'eligible_backends'] = map(_encode_backend, eligible)
199 return data
200
201
202
203=== modified file 'pkgme/tests/test_backend.py'
204--- pkgme/tests/test_backend.py 2012-08-24 13:54:32 +0000
205+++ pkgme/tests/test_backend.py 2012-08-31 10:10:23 +0000
206@@ -368,7 +368,8 @@
207 backends = [backend1, backend2]
208 selector = BackendSelector(backends)
209 self.assertEqual(
210- [(10, backend2), (5, backend1)],
211+ [{'score': 10, 'backend': backend2, 'reason': None},
212+ {'score': 5, 'backend': backend1, 'reason': None}],
213 selector.get_eligible_backends(path))
214
215 def test_order_by_name_after_score(self):
216@@ -381,7 +382,9 @@
217 backends = [backend1, backend2, backend3]
218 selector = BackendSelector(backends)
219 self.assertEqual(
220- [(10, backend2), (10, backend3), (5, backend1)],
221+ [{'score': 10, 'backend': backend2, 'reason': None},
222+ {'score': 10, 'backend': backend3, 'reason': None},
223+ {'score': 5, 'backend': backend1, 'reason': None}],
224 selector.get_eligible_backends(path))
225
226 def test_ineligible_backends_excluded(self):
227@@ -393,7 +396,8 @@
228 backends = [backend1, backend2, backend3]
229 selector = BackendSelector(backends)
230 self.assertEqual(
231- [(10, backend2), (5, backend1)],
232+ [{'score': 10, 'backend': backend2, 'reason': None},
233+ {'score': 5, 'backend': backend1, 'reason': None}],
234 selector.get_eligible_backends(path))
235
236 def test_excludes_non_allowed_backend(self):
237@@ -407,7 +411,8 @@
238 selector = BackendSelector(backends,
239 allowed_backend_names=allowed_backend_names)
240 self.assertEqual(
241- [(10, backend2)], selector.get_eligible_backends(path))
242+ [{'score': 10, 'backend': backend2, 'reason': None}],
243+ selector.get_eligible_backends(path))
244
245
246 class ExternalHelpersBackendLoaderTests(TestCase, TestWithFixtures):
247
248=== modified file 'pkgme/tests/test_script.py'
249--- pkgme/tests/test_script.py 2012-08-06 13:26:01 +0000
250+++ pkgme/tests/test_script.py 2012-08-31 10:10:23 +0000
251@@ -116,6 +116,7 @@
252 signature.close()
253 self.assertNotIn(sig_text, "-----BEGIN PGP SIGNATURE-----\n")
254
255+
256 class GetAllInfoTests(TestCase):
257
258 def test_eligible_backends(self):
259@@ -126,7 +127,9 @@
260 path = self.getUniqueString()
261 info = get_all_info(path, selector)
262 self.assertEqual(
263- [(10, backend2.name), (5, backend1.name)], info['eligible_backends'])
264+ [{'score': 10, 'backend': backend2.name, 'reason': None},
265+ {'score': 5, 'backend': backend1.name, 'reason': None}],
266+ info['eligible_backends'])
267
268 def test_selected_backend(self):
269 info = DictInfo({})

Subscribers

People subscribed via source and target branches