Merge lp:~free.ekanayaka/landscape-client/fix-release-upgrader-not-asking-for-session-id into lp:~landscape/landscape-client/trunk

Proposed by Free Ekanayaka on 2014-12-12
Status: Merged
Approved by: Free Ekanayaka on 2014-12-12
Approved revision: 806
Merged at revision: 805
Proposed branch: lp:~free.ekanayaka/landscape-client/fix-release-upgrader-not-asking-for-session-id
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 77 lines (+14/-6)
3 files modified
landscape/broker/store.py (+1/-1)
landscape/package/releaseupgrader.py (+13/-3)
landscape/package/tests/test_releaseupgrader.py (+0/-2)
To merge this branch: bzr merge lp:~free.ekanayaka/landscape-client/fix-release-upgrader-not-asking-for-session-id
Reviewer Review Type Date Requested Status
Chris Glass (community) 2014-12-12 Approve on 2014-12-12
Björn Tillenius 2014-12-12 Approve on 2014-12-12
Review via email: mp+244573@code.launchpad.net

Commit message

This branch fixes the release upgrader to ask for a session ID before
attempting to send a message.

Unfortunately the test setUp was getting it by hand ignoring that real
code would not do the same.

Description of the change

This branch fixes the release upgrader to ask for a session ID before
attempting to send a message.

Unfortunately the test setUp was getting it by hand ignoring that real
code would not do the same.

To post a comment you must log in.
Björn Tillenius (bjornt) wrote :

Looks good. Minor comment about test coverage, but +1.

review: Approve
Chris Glass (tribaal) wrote :

+1 with Bjorn's comment addressed.

Thanks for jumping on this and fixing the issue so fast :)

review: Approve
806. By Free Ekanayaka on 2014-12-12

Address review comments

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'landscape/broker/store.py'
2--- landscape/broker/store.py 2014-10-03 10:06:24 +0000
3+++ landscape/broker/store.py 2014-12-12 13:25:50 +0000
4@@ -505,7 +505,7 @@
5 # session-ids to one per scope. The or condition here is not
6 # strictly necessary, but we *should* do "is" comparisons when we
7 # can (so says PEP 8).
8- if scope is stored_scope or scope == stored_scope:
9+ if scope == stored_scope:
10 return session_id
11 session_id = str(uuid.uuid4())
12 session_ids[session_id] = scope
13
14=== modified file 'landscape/package/releaseupgrader.py'
15--- landscape/package/releaseupgrader.py 2014-10-29 11:18:17 +0000
16+++ landscape/package/releaseupgrader.py 2014-12-12 13:25:50 +0000
17@@ -82,7 +82,7 @@
18 "The system is already running %s." % target_code_name, 1)
19 logging.info("Queuing message with release upgrade failure to "
20 "exchange urgently.")
21- return self._broker.send_message(message, self._session_id, True)
22+ return self._send_message(message)
23
24 tarball_url = message["upgrade-tool-tarball-url"]
25 signature_url = message["upgrade-tool-signature-url"]
26@@ -232,7 +232,7 @@
27 text, code)
28 logging.info("Queuing message with release upgrade results to "
29 "exchange urgently.")
30- return self._broker.send_message(message, self._session_id, True)
31+ return self._send_message(message)
32
33 result.addCallback(send_operation_result)
34 return result
35@@ -268,12 +268,22 @@
36 logging.info("Queuing message with release upgrade failure to "
37 "exchange urgently.")
38
39- return self._broker.send_message(message, self._session_id, True)
40+ return self._send_message(message)
41
42 @staticmethod
43 def find_command():
44 return find_release_upgrader_command()
45
46+ def _send_message(self, message):
47+ """Acquire a session ID and send the given message."""
48+ deferred = self.get_session_id()
49+
50+ def send(_):
51+ self._broker.send_message(message, self._session_id, True)
52+
53+ deferred.addCallback(send)
54+ return deferred
55+
56
57 def find_release_upgrader_command():
58 """Return the path to the landscape-release-upgrader script."""
59
60=== modified file 'landscape/package/tests/test_releaseupgrader.py'
61--- landscape/package/tests/test_releaseupgrader.py 2014-10-27 09:02:20 +0000
62+++ landscape/package/tests/test_releaseupgrader.py 2014-12-12 13:25:50 +0000
63@@ -2,7 +2,6 @@
64 import unittest
65 import signal
66 import tarfile
67-import ConfigParser
68
69 from twisted.internet import reactor
70 from twisted.internet.defer import succeed, fail, Deferred
71@@ -44,7 +43,6 @@
72 self.store = PackageStore(self.makeFile())
73 self.upgrader = ReleaseUpgrader(self.store, None,
74 self.remote, self.config)
75- self.upgrader.get_session_id()
76 service = self.broker_service
77 service.message_store.set_accepted_types(["operation-result"])
78

Subscribers

People subscribed via source and target branches

to all changes: