Merge lp:~therve/landscape-client/persist-set-intervals into lp:~landscape/landscape-client/trunk

[Thomas Herve]

It's a fairly simple change, passing the config object, and writing it when the message is received.

[Kevin McDermott (bigkevmcd)]

Looks good to me, +1

[Kevin McDermott (bigkevmcd)]

review: Approve

[Frank Wierzbicki (fwierzbicki)]

Looks good to me, +1!

review: Approve

=== modified file 'landscape/broker/exchange.py'
--- landscape/broker/exchange.py	2010-10-25 15:50:29 +0000
+++ landscape/broker/exchange.py	2011-03-25 08:46:40 +0000
@@ -23,8 +23,7 @@
 
     def __init__(self, reactor, store, transport, registration_info,
                  exchange_store,
-                 exchange_interval=60*60,
-                 urgent_exchange_interval=10,
+                 config,
                  monitor_interval=None,
                  max_messages=100,
                  create_time=time.time):
@@ -35,7 +34,7 @@
         @param transport: A L{HTTPTransport} used to deliver messages.
         @param exchange_interval: time interval between subsequent
             exchanges of non-urgent messages.
-        @param urgent_exachange_interval: time interval between subsequent
+        @param urgent_exchange_interval: time interval between subsequent
             exchanges of urgent messages.
         """
         self._reactor = reactor
@@ -43,8 +42,9 @@
         self._create_time = create_time
         self._transport = transport
         self._registration_info = registration_info
-        self._exchange_interval = exchange_interval
-        self._urgent_exchange_interval = urgent_exchange_interval
+        self._config = config
+        self._exchange_interval = config.exchange_interval
+        self._urgent_exchange_interval = config.urgent_exchange_interval
         self._max_messages = max_messages
         self._notification_id = None
         self._exchange_id = None
@@ -159,12 +159,16 @@
     def _handle_set_intervals(self, message):
         if "exchange" in message:
             self._exchange_interval = message["exchange"]
+            self._config.exchange_interval = self._exchange_interval
             logging.info("Exchange interval set to %d seconds." %
                          self._exchange_interval)
         if "urgent-exchange" in message:
             self._urgent_exchange_interval = message["urgent-exchange"]
+            self._config.urgent_exchange_interval = \
+                self._urgent_exchange_interval
             logging.info("Urgent exchange interval set to %d seconds." %
                          self._urgent_exchange_interval)
+        self._config.write()
 
     def exchange(self):
         """Send pending messages to the server and process responses.

=== modified file 'landscape/broker/service.py'
--- landscape/broker/service.py	2010-04-30 10:40:11 +0000
+++ landscape/broker/service.py	2011-03-25 08:46:40 +0000
@@ -56,8 +56,7 @@
         exchange_store = ExchangeStore(self.config.exchange_store_path)
         self.exchanger = MessageExchange(
             self.reactor, self.message_store, self.transport, self.identity,
-            exchange_store, config.exchange_interval,
-            config.urgent_exchange_interval)
+            exchange_store, config)
         self.pinger = self.pinger_factory(self.reactor, config.ping_url,
                                           self.identity, self.exchanger)
         self.registration = RegistrationHandler(

=== modified file 'landscape/broker/tests/helpers.py'
--- landscape/broker/tests/helpers.py	2010-07-19 09:54:58 +0000
+++ landscape/broker/tests/helpers.py	2011-03-25 08:46:40 +0000
@@ -81,9 +81,7 @@
             test_case.config.exchange_store_path)
         test_case.exchanger = MessageExchange(
             test_case.reactor, test_case.mstore, test_case.transport,
-            test_case.identity, test_case.exchange_store,
-            test_case.config.exchange_interval,
-            test_case.config.urgent_exchange_interval)
+            test_case.identity, test_case.exchange_store, test_case.config)
 
 
 class RegistrationHelper(ExchangeHelper):

=== modified file 'landscape/broker/tests/test_exchange.py'
--- landscape/broker/tests/test_exchange.py	2010-10-25 15:50:29 +0000
+++ landscape/broker/tests/test_exchange.py	2011-03-25 08:46:40 +0000
@@ -3,6 +3,7 @@
 from landscape.lib.hashlib import md5
 from landscape.lib.fetch import fetch_async
 from landscape.schema import Message, Int
+from landscape.broker.config import BrokerConfiguration
 from landscape.broker.exchange import get_accepted_types_diff, MessageExchange
 from landscape.broker.transport import FakeTransport
 from landscape.broker.store import MessageStore
@@ -239,6 +240,7 @@
 
         self.mstore.add({"type": "data", "data": 2})
         self.mstore.add({"type": "data", "data": 3})
+
         # next one, server will respond with 1!
         def desynched_send_data(payload, computer_id=None, message_api=None):
             self.transport.next_expected_sequence = 1
@@ -272,7 +274,8 @@
         """
         transport = FakeTransport()
         exchanger = MessageExchange(self.reactor, self.mstore, transport,
-                                    self.identity, self.exchange_store)
+                                    self.identity, self.exchange_store,
+                                    self.config)
         exchanger.start()
         self.wait_for_exchange(urgent=True)
         self.assertEquals(len(transport.payloads), 1)
@@ -303,7 +306,7 @@
         self.wait_for_exchange(urgent=True)
         self.assertEquals(len(self.transport.payloads), 1)
         self.wait_for_exchange(urgent=True)
-        self.assertEquals(len(self.transport.payloads), 1) # no change
+        self.assertEquals(len(self.transport.payloads), 1)  # no change
 
     def test_ancient_causes_resynchronize(self):
         """
@@ -500,7 +503,7 @@
         """
         exchanger = MessageExchange(self.reactor, self.mstore, self.transport,
                                     self.identity, self.exchange_store,
-                                    max_messages=1)
+                                    self.config, max_messages=1)
         self.mstore.set_accepted_types(["empty"])
         self.mstore.add({"type": "empty"})
         self.mstore.add({"type": "empty"})
@@ -528,9 +531,10 @@
         # We create our own MessageExchange because the one set up by the text
         # fixture has an urgent exchange interval of 10 seconds, which makes
         # testing this awkward.
+        self.config.urgent_exchange_interval = 20
         exchanger = MessageExchange(self.reactor, self.mstore, self.transport,
                                     self.identity, self.exchange_store,
-                                    urgent_exchange_interval=20)
+                                    self.config)
         exchanger.schedule_exchange(urgent=True)
         events = []
         self.reactor.call_on("impending-exchange", lambda: events.append(True))
@@ -546,9 +550,11 @@
         should be cancelled and a new one should be scheduled for 10 seconds
         before the new urgent exchange.
         """
+        self.config.exchange_interval = 60 * 60
+        self.config.urgent_exchange_interval = 20
         exchanger = MessageExchange(self.reactor, self.mstore, self.transport,
                                     self.identity, self.exchange_store,
-                                    urgent_exchange_interval=20)
+                                    self.config)
         events = []
         self.reactor.call_on("impending-exchange", lambda: events.append(True))
         # This call will:
@@ -569,9 +575,9 @@
         # schedule a regular exchange.
         # Let's make sure that that *original* impending-exchange event has
         # been cancelled:
-        self.reactor.advance(60 * 60 # time till exchange
-                             - 10 # time till notification
-                             - 20) # time that we've already advanced
+        self.reactor.advance(60 * 60  # time till exchange
+                             - 10  # time till notification
+                             - 20)  # time that we've already advanced
         self.assertEquals(events, [True])
         # Ok, so no new events means that the original call was
         # cancelled. great.
@@ -642,6 +648,11 @@
         self.assertEquals(self.exchanger.get_exchange_intervals(), (60, 900))
 
     def test_set_intervals(self):
+        """
+        When a C{set-intervals} message is received, the runtime attributes of
+        the L{MessageExchange} are changed, the configuration values as well,
+        and the configuration is written to disk to be persisted.
+        """
         server_message = [{"type": "set-intervals",
                            "urgent-exchange": 1234, "exchange": 5678}]
         self.transport.responses.append(server_message)
@@ -650,6 +661,13 @@
 
         self.assertEquals(self.exchanger.get_exchange_intervals(),
                           (1234, 5678))
+        self.assertEquals(self.config.exchange_interval, 5678)
+        self.assertEquals(self.config.urgent_exchange_interval, 1234)
+
+        new_config = BrokerConfiguration()
+        new_config.load_configuration_file(self.config_filename)
+        self.assertEquals(new_config.exchange_interval, 5678)
+        self.assertEquals(new_config.urgent_exchange_interval, 1234)
 
     def test_set_intervals_with_urgent_exchange_only(self):
         server_message = [{"type": "set-intervals", "urgent-exchange": 1234}]