Merge lp:~fcorrea/landscape-client/retry-apt-update into lp:~landscape/landscape-client/trunk
- retry-apt-update
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Fernando Correa Neto |
Approved revision: | 815 |
Merged at revision: | 808 |
Proposed branch: | lp:~fcorrea/landscape-client/retry-apt-update |
Merge into: | lp:~landscape/landscape-client/trunk |
Diff against target: |
522 lines (+169/-66) 6 files modified
landscape/package/changer.py (+2/-1) landscape/package/reporter.py (+40/-16) landscape/package/taskhandler.py (+4/-2) landscape/package/tests/test_releaseupgrader.py (+1/-1) landscape/package/tests/test_reporter.py (+117/-43) landscape/package/tests/test_taskhandler.py (+5/-3) |
To merge this branch: | bzr merge lp:~fcorrea/landscape-client/retry-apt-update |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Данило Шеган (community) | Approve | ||
Free Ekanayaka (community) | Approve | ||
Review via email: mp+245734@code.launchpad.net |
Commit message
Change the package reporter so it retries running apt-get update when an error 100 is returned, pretty much in the same way the charm code does it.
Description of the change
This branch changes the package reporter so it retries running apt-get update when an error 100 is returned, pretty much in the same way the charm code does it.
- 808. By Fernando Correa Neto <email address hidden>
-
- change task handler code to accept a reactor.
- adjust tests to deal with it - 809. By Fernando Correa Neto <email address hidden>
-
- adjust tests to advance the reactor manually
- 810. By Fernando Correa Neto <email address hidden>
-
- fix tests to comply with the new method signature
- simplify existing apt_update tests and so that it never messes with the real reactor - 811. By Fernando Correa Neto <email address hidden>
-
- lint
- 812. By Fernando Correa Neto <email address hidden>
-
- do not gracefully bail. Log a warning instead
Fernando Correa Neto (fcorrea) wrote : | # |
Thanks for the review Free.
I've addressed all points but one. See bellow.
Also, instead of faking the spawn_process call, I've decided to mock it as it ended up being cleaner.
Free Ekanayaka (free.ekanayaka) wrote : | # |
Thanks Fernando, replied to the pending point and adding another one.
Free Ekanayaka (free.ekanayaka) : | # |
- 813. By Fernando Correa Neto <email address hidden>
-
- address review comments
Fernando Correa Neto (fcorrea) wrote : | # |
Thanks, Free.
All comments has been addressed.
Free Ekanayaka (free.ekanayaka) wrote : | # |
Looks great thanks, one last small thing, see the comment. +1
Данило Шеган (danilo) wrote : | # |
Looks good, two minor comments inline.
Preview Diff
1 | === modified file 'landscape/package/changer.py' |
2 | --- landscape/package/changer.py 2013-07-16 13:15:03 +0000 |
3 | +++ landscape/package/changer.py 2015-01-12 13:35:57 +0000 |
4 | @@ -66,7 +66,8 @@ |
5 | def __init__(self, store, facade, remote, config, process_factory=reactor, |
6 | landscape_reactor=None, |
7 | reboot_required_filename=REBOOT_REQUIRED_FILENAME): |
8 | - super(PackageChanger, self).__init__(store, facade, remote, config) |
9 | + super(PackageChanger, self).__init__( |
10 | + store, facade, remote, config, landscape_reactor) |
11 | self._process_factory = process_factory |
12 | if landscape_reactor is None: # For testing purposes. |
13 | from landscape.reactor import LandscapeReactor |
14 | |
15 | === modified file 'landscape/package/reporter.py' |
16 | --- landscape/package/reporter.py 2014-09-24 14:48:40 +0000 |
17 | +++ landscape/package/reporter.py 2015-01-12 13:35:57 +0000 |
18 | @@ -6,7 +6,8 @@ |
19 | import glob |
20 | import apt_pkg |
21 | |
22 | -from twisted.internet.defer import Deferred, succeed |
23 | +from twisted.internet.defer import ( |
24 | + Deferred, succeed, inlineCallbacks, returnValue) |
25 | |
26 | from landscape.lib.sequenceranges import sequence_to_ranges |
27 | from landscape.lib.twisted_util import gather_results, spawn_process |
28 | @@ -21,6 +22,7 @@ |
29 | |
30 | HASH_ID_REQUEST_TIMEOUT = 7200 |
31 | MAX_UNKNOWN_HASHES_PER_REQUEST = 500 |
32 | +LOCK_RETRY_DELAYS = [0, 20, 40] |
33 | |
34 | |
35 | class PackageReporterConfiguration(PackageTaskHandlerConfiguration): |
36 | @@ -192,8 +194,11 @@ |
37 | last_update = os.stat(stamp).st_mtime |
38 | return (last_update + interval) < time.time() |
39 | |
40 | + @inlineCallbacks |
41 | def run_apt_update(self): |
42 | - """Run apt-update and log a warning in case of non-zero exit code. |
43 | + """ |
44 | + Check if an L{_apt_update} call must be performed looping over specific |
45 | + delays so it can be retried. |
46 | |
47 | @return: a deferred returning (out, err, code) |
48 | """ |
49 | @@ -201,12 +206,15 @@ |
50 | or self._apt_update_timeout_expired( |
51 | self._config.apt_update_interval)): |
52 | |
53 | - result = spawn_process(self.apt_update_filename) |
54 | + accepted_apt_errors = ( |
55 | + "Problem renaming the file /var/cache/apt/srcpkgcache.bin", |
56 | + "Problem renaming the file /var/cache/apt/pkgcache.bin") |
57 | |
58 | - def callback((out, err, code)): |
59 | - accepted_apt_errors = ( |
60 | - "Problem renaming the file /var/cache/apt/srcpkgcache.bin", |
61 | - "Problem renaming the file /var/cache/apt/pkgcache.bin") |
62 | + for retry in range(len(LOCK_RETRY_DELAYS)): |
63 | + deferred = Deferred() |
64 | + self._reactor.call_later( |
65 | + LOCK_RETRY_DELAYS[retry], self._apt_update, deferred) |
66 | + out, err, code = yield deferred |
67 | |
68 | touch_file(self._config.update_stamp_filename) |
69 | logging.debug( |
70 | @@ -214,11 +222,19 @@ |
71 | self.apt_update_filename, code, out, err)) |
72 | |
73 | if code != 0: |
74 | + if code == 100: |
75 | + if retry < len(LOCK_RETRY_DELAYS) - 1: |
76 | + logging.warning( |
77 | + "Could not acquire the apt lock. Retrying in" |
78 | + " %s seconds." % LOCK_RETRY_DELAYS[retry + 1]) |
79 | + continue |
80 | + |
81 | logging.warning("'%s' exited with status %d (%s)" % ( |
82 | self.apt_update_filename, code, err)) |
83 | |
84 | - # Errors caused by missing cache files are acceptable, as |
85 | - # they are not an issue for the lists update process. |
86 | + # Errors caused by missing cache files are acceptable, |
87 | + # as they are not an issue for the lists update |
88 | + # process. |
89 | # These errors can happen if an 'apt-get clean' is run |
90 | # while 'apt-get update' is running. |
91 | for message in accepted_apt_errors: |
92 | @@ -232,17 +248,25 @@ |
93 | (self.sources_list_filename, |
94 | self.sources_list_directory)) |
95 | |
96 | - deferred = self._broker.call_if_accepted( |
97 | + yield self._broker.call_if_accepted( |
98 | "package-reporter-result", self.send_result, code, err) |
99 | - deferred.addCallback(lambda ignore: (out, err, code)) |
100 | - return deferred |
101 | - |
102 | - return result.addCallback(callback) |
103 | - |
104 | + yield returnValue((out, err, code)) |
105 | else: |
106 | logging.debug("'%s' didn't run, update interval has not passed" % |
107 | self.apt_update_filename) |
108 | - return succeed(("", "", 0)) |
109 | + yield returnValue(("", "", 0)) |
110 | + |
111 | + def _apt_update(self, deferred): |
112 | + """ |
113 | + Run apt-update using the passed in deferred, which allows for callers |
114 | + to inspect the result code. |
115 | + """ |
116 | + result = spawn_process(self.apt_update_filename) |
117 | + |
118 | + def callback((out, err, code), deferred): |
119 | + return deferred.callback((out, err, code)) |
120 | + |
121 | + return result.addCallback(callback, deferred) |
122 | |
123 | def send_result(self, code, err): |
124 | """ |
125 | |
126 | === modified file 'landscape/package/taskhandler.py' |
127 | --- landscape/package/taskhandler.py 2013-05-21 08:37:08 +0000 |
128 | +++ landscape/package/taskhandler.py 2015-01-12 13:35:57 +0000 |
129 | @@ -97,13 +97,15 @@ |
130 | # update-notifier-common package is installed. |
131 | update_notifier_stamp = "/var/lib/apt/periodic/update-success-stamp" |
132 | |
133 | - def __init__(self, package_store, package_facade, remote_broker, config): |
134 | + def __init__(self, package_store, package_facade, remote_broker, config, |
135 | + reactor): |
136 | self._store = package_store |
137 | self._facade = package_facade |
138 | self._broker = remote_broker |
139 | self._config = config |
140 | self._count = 0 |
141 | self._session_id = None |
142 | + self._reactor = reactor |
143 | |
144 | def run(self): |
145 | return self.handle_tasks() |
146 | @@ -295,7 +297,7 @@ |
147 | |
148 | connector = RemoteBrokerConnector(reactor, config, retry_on_reconnect=True) |
149 | remote = LazyRemoteBroker(connector) |
150 | - handler = cls(package_store, package_facade, remote, config) |
151 | + handler = cls(package_store, package_facade, remote, config, reactor) |
152 | result = Deferred() |
153 | result.addCallback(lambda x: handler.run()) |
154 | result.addCallback(lambda x: finish()) |
155 | |
156 | === modified file 'landscape/package/tests/test_releaseupgrader.py' |
157 | --- landscape/package/tests/test_releaseupgrader.py 2014-12-12 12:39:21 +0000 |
158 | +++ landscape/package/tests/test_releaseupgrader.py 2015-01-12 13:35:57 +0000 |
159 | @@ -42,7 +42,7 @@ |
160 | os.mkdir(self.config.upgrade_tool_directory) |
161 | self.store = PackageStore(self.makeFile()) |
162 | self.upgrader = ReleaseUpgrader(self.store, None, |
163 | - self.remote, self.config) |
164 | + self.remote, self.config, None) |
165 | service = self.broker_service |
166 | service.message_store.set_accepted_types(["operation-result"]) |
167 | |
168 | |
169 | === modified file 'landscape/package/tests/test_reporter.py' |
170 | --- landscape/package/tests/test_reporter.py 2014-09-24 14:48:40 +0000 |
171 | +++ landscape/package/tests/test_reporter.py 2015-01-12 13:35:57 +0000 |
172 | @@ -23,6 +23,7 @@ |
173 | from landscape.tests.helpers import ( |
174 | LandscapeTest, BrokerServiceHelper, EnvironSaverHelper) |
175 | from landscape.tests.mocker import ANY |
176 | +from landscape.reactor import FakeReactor |
177 | |
178 | SAMPLE_LSB_RELEASE = "DISTRIB_CODENAME=codename\n" |
179 | |
180 | @@ -51,8 +52,9 @@ |
181 | super(PackageReporterAptTest, self).setUp() |
182 | self.store = PackageStore(self.makeFile()) |
183 | self.config = PackageReporterConfiguration() |
184 | + self.reactor = FakeReactor() |
185 | self.reporter = PackageReporter( |
186 | - self.store, self.facade, self.remote, self.config) |
187 | + self.store, self.facade, self.remote, self.config, self.reactor) |
188 | self.reporter.get_session_id() |
189 | # Assume update-notifier-common stamp file is not present by |
190 | # default. |
191 | @@ -1119,7 +1121,6 @@ |
192 | deferred = self.reporter.run() |
193 | |
194 | def check_result(result): |
195 | - |
196 | # The hashes should not go away. |
197 | hash1 = self.store.get_hash_id(foo_hash) |
198 | hash2 = self.store.get_hash_id(HASH2) |
199 | @@ -1140,6 +1141,7 @@ |
200 | self.assertEqual(request.hashes, [HASH3, HASH1]) |
201 | |
202 | deferred.addCallback(check_result) |
203 | + self.reactor.advance(0) |
204 | return deferred |
205 | |
206 | def test_run_apt_update(self): |
207 | @@ -1165,6 +1167,7 @@ |
208 | self.assertEqual("error", err) |
209 | self.assertEqual(0, code) |
210 | result.addCallback(callback) |
211 | + self.reactor.advance(0) |
212 | result.chainDeferred(deferred) |
213 | |
214 | reactor.callWhenRunning(do_test) |
215 | @@ -1179,18 +1182,14 @@ |
216 | self.config.load(["--force-apt-update"]) |
217 | self._make_fake_apt_update() |
218 | |
219 | - deferred = Deferred() |
220 | - |
221 | - def do_test(): |
222 | - result = self.reporter.run_apt_update() |
223 | - |
224 | - def callback((out, err, code)): |
225 | - self.assertEqual("output", out) |
226 | - result.addCallback(callback) |
227 | - result.chainDeferred(deferred) |
228 | - |
229 | - reactor.callWhenRunning(do_test) |
230 | - return deferred |
231 | + result = self.reporter.run_apt_update() |
232 | + |
233 | + def callback((out, err, code)): |
234 | + self.assertEqual("output", out) |
235 | + |
236 | + result.addCallback(callback) |
237 | + self.reactor.advance(0) |
238 | + return result |
239 | |
240 | def test_run_apt_update_with_force_apt_update_if_sources_changed(self): |
241 | """ |
242 | @@ -1203,18 +1202,14 @@ |
243 | self.reporter.sources_list_filename = self.makeFile("deb ftp://url ./") |
244 | self._make_fake_apt_update() |
245 | |
246 | - deferred = Deferred() |
247 | - |
248 | - def do_test(): |
249 | - result = self.reporter.run_apt_update() |
250 | - |
251 | - def callback((out, err, code)): |
252 | - self.assertEqual("output", out) |
253 | - result.addCallback(callback) |
254 | - result.chainDeferred(deferred) |
255 | - |
256 | - reactor.callWhenRunning(do_test) |
257 | - return deferred |
258 | + result = self.reporter.run_apt_update() |
259 | + |
260 | + def callback((out, err, code)): |
261 | + self.assertEqual("output", out) |
262 | + |
263 | + result.addCallback(callback) |
264 | + self.reactor.advance(0) |
265 | + return result |
266 | |
267 | def test_run_apt_update_warns_about_failures(self): |
268 | """ |
269 | @@ -1225,21 +1220,87 @@ |
270 | logging_mock = self.mocker.replace("logging.warning") |
271 | logging_mock("'%s' exited with status 2" |
272 | " (error)" % self.reporter.apt_update_filename) |
273 | - self.mocker.replay() |
274 | - deferred = Deferred() |
275 | - |
276 | - def do_test(): |
277 | - result = self.reporter.run_apt_update() |
278 | - |
279 | - def callback((out, err, code)): |
280 | - self.assertEqual("output", out) |
281 | - self.assertEqual("error", err) |
282 | - self.assertEqual(2, code) |
283 | - result.addCallback(callback) |
284 | - result.chainDeferred(deferred) |
285 | - |
286 | - reactor.callWhenRunning(do_test) |
287 | - return deferred |
288 | + |
289 | + self.mocker.replay() |
290 | + |
291 | + result = self.reporter.run_apt_update() |
292 | + |
293 | + def callback((out, err, code)): |
294 | + self.assertEqual("output", out) |
295 | + self.assertEqual("error", err) |
296 | + self.assertEqual(2, code) |
297 | + |
298 | + result.addCallback(callback) |
299 | + self.reactor.advance(0) |
300 | + return result |
301 | + |
302 | + def test_run_apt_update_warns_about_lock_failure(self): |
303 | + """ |
304 | + The L{PackageReporter.run_apt_update} method logs a warnings when |
305 | + apt-update fails acquiring the lock. |
306 | + """ |
307 | + self._make_fake_apt_update(code=100) |
308 | + logging_mock = self.mocker.replace("logging.warning") |
309 | + logging_mock("Could not acquire the apt lock. Retrying in 20 seconds.") |
310 | + logging_mock("Could not acquire the apt lock. Retrying in 40 seconds.") |
311 | + logging_mock("'%s' exited with status 100 ()" % |
312 | + self.reporter.apt_update_filename) |
313 | + |
314 | + spawn_mock = self.mocker.replace( |
315 | + "landscape.lib.twisted_util.spawn_process") |
316 | + spawn_mock(ANY) |
317 | + # Simulate the first failure. |
318 | + self.mocker.result(succeed(('', '', 100))) |
319 | + spawn_mock(ANY) |
320 | + # Simulate the second failure. |
321 | + self.mocker.result(succeed(('', '', 100))) |
322 | + spawn_mock(ANY) |
323 | + # Simulate the second failure. |
324 | + self.mocker.result(succeed(('', '', 100))) |
325 | + |
326 | + self.mocker.replay() |
327 | + |
328 | + result = self.reporter.run_apt_update() |
329 | + |
330 | + def callback((out, err, code)): |
331 | + self.assertEqual("", out) |
332 | + self.assertEqual("", err) |
333 | + self.assertEqual(100, code) |
334 | + |
335 | + result.addCallback(callback) |
336 | + self.reactor.advance(60) |
337 | + return result |
338 | + |
339 | + def test_run_apt_update_stops_retrying_after_lock_acquired(self): |
340 | + """ |
341 | + When L{PackageReporter.run_apt_update} method successfully acquires the |
342 | + lock, it will stop retrying. |
343 | + """ |
344 | + self._make_fake_apt_update(code=100) |
345 | + logging_mock = self.mocker.replace("logging.warning") |
346 | + logging_mock("Could not acquire the apt lock. Retrying in 20 seconds.") |
347 | + |
348 | + spawn_mock = self.mocker.replace( |
349 | + "landscape.lib.twisted_util.spawn_process") |
350 | + spawn_mock(ANY) |
351 | + # Simulate the first failure. |
352 | + self.mocker.result(succeed(('', '', 100))) |
353 | + spawn_mock(ANY) |
354 | + # Simulate a successful apt lock grab. |
355 | + self.mocker.result(succeed(('output', 'error', 0))) |
356 | + |
357 | + self.mocker.replay() |
358 | + |
359 | + result = self.reporter.run_apt_update() |
360 | + |
361 | + def callback((out, err, code)): |
362 | + self.assertEqual("output", out) |
363 | + self.assertEqual("error", err) |
364 | + self.assertEqual(0, code) |
365 | + |
366 | + result.addCallback(callback) |
367 | + self.reactor.advance(20) |
368 | + return result |
369 | |
370 | def test_run_apt_update_report_apt_failure(self): |
371 | """ |
372 | @@ -1260,6 +1321,7 @@ |
373 | [{"type": "package-reporter-result", |
374 | "code": 2, "err": u"error"}]) |
375 | result.addCallback(callback) |
376 | + self.reactor.advance(0) |
377 | result.chainDeferred(deferred) |
378 | |
379 | reactor.callWhenRunning(do_test) |
380 | @@ -1290,6 +1352,7 @@ |
381 | [{"type": "package-reporter-result", |
382 | "code": 1, "err": error}]) |
383 | result.addCallback(callback) |
384 | + self.reactor.advance(0) |
385 | result.chainDeferred(deferred) |
386 | |
387 | reactor.callWhenRunning(do_test) |
388 | @@ -1315,6 +1378,7 @@ |
389 | [{"type": "package-reporter-result", |
390 | "code": 2, "err": u"error"}]) |
391 | result.addCallback(callback) |
392 | + self.reactor.advance(0) |
393 | result.chainDeferred(deferred) |
394 | |
395 | reactor.callWhenRunning(do_test) |
396 | @@ -1339,6 +1403,7 @@ |
397 | [{"type": "package-reporter-result", |
398 | "code": 0, "err": u"message"}]) |
399 | result.addCallback(callback) |
400 | + self.reactor.advance(0) |
401 | result.chainDeferred(deferred) |
402 | |
403 | reactor.callWhenRunning(do_test) |
404 | @@ -1366,6 +1431,7 @@ |
405 | self.assertEqual("", err) |
406 | self.assertEqual(0, code) |
407 | result.addCallback(callback) |
408 | + self.reactor.advance(0) |
409 | result.chainDeferred(deferred) |
410 | |
411 | reactor.callWhenRunning(do_test) |
412 | @@ -1402,6 +1468,7 @@ |
413 | self.assertEqual("", err) |
414 | self.assertEqual(0, code) |
415 | result.addCallback(callback) |
416 | + self.reactor.advance(0) |
417 | result.chainDeferred(deferred) |
418 | |
419 | reactor.callWhenRunning(do_test) |
420 | @@ -1438,6 +1505,7 @@ |
421 | [{"type": "package-reporter-result", |
422 | "code": 0, "err": u"message"}]) |
423 | result.addCallback(callback) |
424 | + self.reactor.advance(0) |
425 | result.chainDeferred(deferred) |
426 | |
427 | reactor.callWhenRunning(do_test) |
428 | @@ -1459,6 +1527,7 @@ |
429 | self.assertTrue( |
430 | os.path.exists(self.config.update_stamp_filename)) |
431 | result.addCallback(callback) |
432 | + self.reactor.advance(0) |
433 | result.chainDeferred(deferred) |
434 | |
435 | reactor.callWhenRunning(do_test) |
436 | @@ -1492,6 +1561,7 @@ |
437 | [{"type": "package-reporter-result", |
438 | "code": 0, "err": u""}]) |
439 | result.addCallback(callback) |
440 | + self.reactor.advance(0) |
441 | result.chainDeferred(deferred) |
442 | |
443 | reactor.callWhenRunning(do_test) |
444 | @@ -1528,6 +1598,7 @@ |
445 | [{"type": "package-reporter-result", |
446 | "code": 0, "err": u""}]) |
447 | result.addCallback(callback) |
448 | + self.reactor.advance(0) |
449 | result.chainDeferred(deferred) |
450 | |
451 | reactor.callWhenRunning(do_test) |
452 | @@ -1632,8 +1703,9 @@ |
453 | super(GlobalPackageReporterAptTest, self).setUp() |
454 | self.store = FakePackageStore(self.makeFile()) |
455 | self.config = PackageReporterConfiguration() |
456 | + self.reactor = FakeReactor() |
457 | self.reporter = FakeGlobalReporter( |
458 | - self.store, self.facade, self.remote, self.config) |
459 | + self.store, self.facade, self.remote, self.config, self.reactor) |
460 | # Assume update-notifier-common stamp file is not present by |
461 | # default. |
462 | self.reporter.update_notifier_stamp = "/Not/Existing" |
463 | @@ -1654,6 +1726,7 @@ |
464 | def do_test(): |
465 | self.reporter.get_session_id() |
466 | result = self.reporter.run_apt_update() |
467 | + self.reactor.advance(0) |
468 | |
469 | def callback(ignore): |
470 | message = {"type": "package-reporter-result", |
471 | @@ -1683,8 +1756,9 @@ |
472 | self.global_store = FakePackageStore(global_file) |
473 | os.environ["FAKE_PACKAGE_STORE"] = global_file |
474 | self.config = PackageReporterConfiguration() |
475 | + self.reactor = FakeReactor() |
476 | self.reporter = FakeReporter( |
477 | - self.store, None, self.remote, self.config) |
478 | + self.store, None, self.remote, self.config, self.reactor) |
479 | self.config.data_path = self.makeDir() |
480 | os.mkdir(self.config.package_directory) |
481 | |
482 | |
483 | === modified file 'landscape/package/tests/test_taskhandler.py' |
484 | --- landscape/package/tests/test_taskhandler.py 2013-05-21 08:37:08 +0000 |
485 | +++ landscape/package/tests/test_taskhandler.py 2015-01-12 13:35:57 +0000 |
486 | @@ -44,8 +44,9 @@ |
487 | super(PackageTaskHandlerTest, self).setUp() |
488 | self.config = PackageTaskHandlerConfiguration() |
489 | self.store = PackageStore(self.makeFile()) |
490 | + self.reactor = FakeReactor() |
491 | self.handler = PackageTaskHandler( |
492 | - self.store, self.facade, self.remote, self.config) |
493 | + self.store, self.facade, self.remote, self.config, self.reactor) |
494 | |
495 | def test_use_hash_id_db(self): |
496 | |
497 | @@ -363,7 +364,7 @@ |
498 | umask(022) |
499 | |
500 | handler_args = [] |
501 | - HandlerMock(ANY, ANY, ANY, ANY) |
502 | + HandlerMock(ANY, ANY, ANY, ANY, ANY) |
503 | self.mocker.passthrough() # Let the real constructor run for testing. |
504 | self.mocker.call(lambda *args: handler_args.extend(args)) |
505 | |
506 | @@ -390,7 +391,7 @@ |
507 | |
508 | def assert_task_handler(ignored): |
509 | |
510 | - store, facade, broker, config = handler_args |
511 | + store, facade, broker, config, reactor = handler_args |
512 | |
513 | try: |
514 | # Verify the arguments passed to the reporter constructor. |
515 | @@ -399,6 +400,7 @@ |
516 | self.assertEqual(type(broker), LazyRemoteBroker) |
517 | self.assertEqual(type(config), |
518 | PackageTaskHandlerConfiguration) |
519 | + self.assertEqual(type(reactor), LandscapeReactor) |
520 | |
521 | # Let's see if the store path is where it should be. |
522 | filename = os.path.join(self.data_path, "package", "database") |
Looks basically good, just a few comments about testing and small factoring things.