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

[Free Ekanayaka]

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.

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.

[Björn Tillenius (bjornt)]

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

review: Approve

[Chris Glass (tribaal)]

+1 with Bjorn's comment addressed.

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

review: Approve

[lp:~free.ekanayaka/landscape-client/fix-release-upgrader-not-asking-for-session-id]

806. By Free Ekanayaka on 2014-12-12

Address review comments

=== modified file 'landscape/broker/store.py'
--- landscape/broker/store.py	2014-10-03 10:06:24 +0000
+++ landscape/broker/store.py	2014-12-12 13:25:50 +0000
@@ -505,7 +505,7 @@
             # session-ids to one per scope.  The or condition here is not
             # strictly necessary, but we *should* do "is" comparisons when we
             # can (so says PEP 8).
-            if scope is stored_scope or scope == stored_scope:
+            if scope == stored_scope:
                 return session_id
         session_id = str(uuid.uuid4())
         session_ids[session_id] = scope
 
=== modified file 'landscape/package/releaseupgrader.py'
--- landscape/package/releaseupgrader.py	2014-10-29 11:18:17 +0000
+++ landscape/package/releaseupgrader.py	2014-12-12 13:25:50 +0000
@@ -82,7 +82,7 @@
                 "The system is already running %s." % target_code_name, 1)
             logging.info("Queuing message with release upgrade failure to "
                          "exchange urgently.")
-            return self._broker.send_message(message, self._session_id, True)
+            return self._send_message(message)
 
         tarball_url = message["upgrade-tool-tarball-url"]
         signature_url = message["upgrade-tool-signature-url"]
@@ -232,7 +232,7 @@
                                                          text, code)
             logging.info("Queuing message with release upgrade results to "
                          "exchange urgently.")
-            return self._broker.send_message(message, self._session_id, True)
+            return self._send_message(message)
 
         result.addCallback(send_operation_result)
         return result
@@ -268,12 +268,22 @@
         logging.info("Queuing message with release upgrade failure to "
                      "exchange urgently.")
 
-        return self._broker.send_message(message, self._session_id, True)
+        return self._send_message(message)
 
     @staticmethod
     def find_command():
         return find_release_upgrader_command()
 
+    def _send_message(self, message):
+        """Acquire a session ID and send the given message."""
+        deferred = self.get_session_id()
+
+        def send(_):
+            self._broker.send_message(message, self._session_id, True)
+
+        deferred.addCallback(send)
+        return deferred
+
 
 def find_release_upgrader_command():
     """Return the path to the landscape-release-upgrader script."""
 
=== modified file 'landscape/package/tests/test_releaseupgrader.py'
--- landscape/package/tests/test_releaseupgrader.py	2014-10-27 09:02:20 +0000
+++ landscape/package/tests/test_releaseupgrader.py	2014-12-12 13:25:50 +0000
@@ -2,7 +2,6 @@
 import unittest
 import signal
 import tarfile
-import ConfigParser
 
 from twisted.internet import reactor
 from twisted.internet.defer import succeed, fail, Deferred
@@ -44,7 +43,6 @@
         self.store = PackageStore(self.makeFile())
         self.upgrader = ReleaseUpgrader(self.store, None,
                                         self.remote, self.config)
-        self.upgrader.get_session_id()
         service = self.broker_service
         service.message_store.set_accepted_types(["operation-result"])