Merge lp:~therve/landscape-client/package-reporter-result into lp:~landscape/landscape-client/trunk

Proposed by Thomas Herve
Status: Merged
Approved by: Free Ekanayaka
Approved revision: 330
Merged at revision: 329
Proposed branch: lp:~therve/landscape-client/package-reporter-result
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 111 lines (+39/-18)
3 files modified
landscape/message_schemas.py (+4/-4)
landscape/package/reporter.py (+7/-11)
landscape/package/tests/test_reporter.py (+28/-3)
To merge this branch: bzr merge lp:~therve/landscape-client/package-reporter-result
Reviewer Review Type Date Requested Status
Free Ekanayaka (community) Approve
Sidnei da Silva (community) Approve
Review via email: mp+60474@code.launchpad.net

Description of the change

The branch makes the client reports *all* results from smart update, allowing
to correctly know when the client is correctly working or not.

To post a comment you must log in.
Revision history for this message
Sidnei da Silva (sidnei) wrote :

LGTM. +1!

review: Approve
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Great, +1!

[1]

+ "out": out,

The output could be biggish, not sure that we really want to include it in the message, also it's not clear what use the server would make of it. Maybe it could be skipped initially (till we find a use case)? IIRC adding a field to a message shouldn't be painful. This is not blocking if you think that the extra payload is negligible.

review: Approve
331. By Thomas Herve

Remove output from the package reporter result

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Verified in staging, with a staging client.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'landscape/message_schemas.py'
2--- landscape/message_schemas.py 2011-04-29 16:23:16 +0000
3+++ landscape/message_schemas.py 2011-05-12 10:22:26 +0000
4@@ -297,9 +297,9 @@
5 "request-id": Int(),
6 })
7
8-PACKAGE_REPORTER_ERROR = Message("package-reporter-error", {
9- "error-code": Int(),
10- "error-text": utf8})
11+PACKAGE_REPORTER_RESULT = Message("package-reporter-result", {
12+ "code": Int(),
13+ "err": utf8})
14
15 ADD_PACKAGES = Message("add-packages", {
16 "packages": List(KeyDict({"name": utf8,
17@@ -390,7 +390,7 @@
18 REGISTER, REGISTER_CLOUD_VM, TEMPERATURE, PROCESSOR_INFO,
19 USERS, PACKAGES, PACKAGE_LOCKS,
20 CHANGE_PACKAGES_RESULT, UNKNOWN_PACKAGE_HASHES,
21- ADD_PACKAGES, PACKAGE_REPORTER_ERROR, TEXT_MESSAGE, TEST,
22+ ADD_PACKAGES, PACKAGE_REPORTER_RESULT, TEXT_MESSAGE, TEST,
23 CUSTOM_GRAPH, REBOOT_REQUIRED, APT_PREFERENCES, EUCALYPTUS_INFO,
24 EUCALYPTUS_INFO_ERROR, NETWORK_DEVICE, NETWORK_ACTIVITY,
25 REBOOT_REQUIRED_INFO]:
26
27=== modified file 'landscape/package/reporter.py'
28--- landscape/package/reporter.py 2011-04-29 16:23:16 +0000
29+++ landscape/package/reporter.py 2011-05-12 10:22:26 +0000
30@@ -201,26 +201,22 @@
31 logging.debug("'%s' exited with status %d (out='%s', err='%s'" % (
32 self.smart_update_filename, code, out, err))
33 touch_file(self._config.smart_update_stamp_filename)
34- if smart_failed:
35- deferred = self._broker.call_if_accepted(
36- "package-reporter-error", self.send_error, code, err)
37- else:
38- deferred = succeed(None)
39+ deferred = self._broker.call_if_accepted(
40+ "package-reporter-result", self.send_result, code, err)
41 deferred.addCallback(lambda ignore: (out, err, code))
42 return deferred
43
44 result.addCallback(callback)
45 return result
46
47- def send_error(self, code, err):
48+ def send_result(self, code, err):
49 """
50- If an error happened in smart update, reports it to the server in a
51- message.
52+ Report the package reporter result to the server in a message.
53 """
54 message = {
55- "type": "package-reporter-error",
56- "error-code": code,
57- "error-text": err}
58+ "type": "package-reporter-result",
59+ "code": code,
60+ "err": err}
61 return self._broker.send_message(message, True)
62
63 def handle_task(self, task):
64
65=== modified file 'landscape/package/tests/test_reporter.py'
66--- landscape/package/tests/test_reporter.py 2011-04-29 16:23:16 +0000
67+++ landscape/package/tests/test_reporter.py 2011-05-12 10:22:26 +0000
68@@ -668,7 +668,7 @@
69 server reporting the error, to be able to fix the problem centrally.
70 """
71 message_store = self.broker_service.message_store
72- message_store.set_accepted_types(["package-reporter-error"])
73+ message_store.set_accepted_types(["package-reporter-result"])
74 self.reporter.smart_update_filename = self.makeFile(
75 "#!/bin/sh\necho -n error >&2\necho -n output\nexit 2")
76 os.chmod(self.reporter.smart_update_filename, 0755)
77@@ -679,8 +679,33 @@
78
79 def callback(ignore):
80 self.assertMessages(message_store.get_pending_messages(),
81- [{"type": "package-reporter-error",
82- "error-code": 2, "error-text": "error"}])
83+ [{"type": "package-reporter-result",
84+ "code": 2, "err": u"error"}])
85+ result.addCallback(callback)
86+ result.chainDeferred(deferred)
87+
88+ reactor.callWhenRunning(do_test)
89+ return deferred
90+
91+ def test_run_smart_update_report_success(self):
92+ """
93+ L{PackageReporter.run_smart_update} also reports success to be able to
94+ know the proper state of the client.
95+ """
96+ message_store = self.broker_service.message_store
97+ message_store.set_accepted_types(["package-reporter-result"])
98+ self.reporter.smart_update_filename = self.makeFile(
99+ "#!/bin/sh\necho -n error >&2\necho -n output\nexit 0")
100+ os.chmod(self.reporter.smart_update_filename, 0755)
101+ deferred = Deferred()
102+
103+ def do_test():
104+ result = self.reporter.run_smart_update()
105+
106+ def callback(ignore):
107+ self.assertMessages(message_store.get_pending_messages(),
108+ [{"type": "package-reporter-result",
109+ "code": 0, "err": u"error"}])
110 result.addCallback(callback)
111 result.chainDeferred(deferred)
112

Subscribers

People subscribed via source and target branches

to all changes: