Merge lp:~facundo/magicicada-gui/retry-dbus into lp:magicicada-gui

[Facundo Batista]

DBus calls are retryable now!

[lp:~facundo/magicicada-gui/retry-dbus]

38. By Facundo Batista on 2010-06-04

Removed debugging prints, and fixed some docstrings.

[Natalia Bidart (nataliabidart)]

Perfect!

review: Approve

=== modified file 'magicicada/dbusiface.py'
--- magicicada/dbusiface.py	2010-05-31 16:45:45 +0000
+++ magicicada/dbusiface.py	2010-06-04 02:47:22 +0000
@@ -26,6 +26,7 @@
 import dbus
 from dbus import SessionBus
 from dbus.mainloop.glib import DBusGMainLoop
+from twisted.internet import defer
 
 from ubuntuone.syncdaemon.tools import SyncDaemonTool
 
@@ -59,6 +60,34 @@
                 "new_parent_id=(.*?), new_name=(.*?)\)")
 
 
+def _is_retry_exception(err):
+    """Check if the exception is a retry one."""
+    if isinstance(err, dbus.exceptions.DBusException):
+        if err.get_dbus_name() == 'org.freedesktop.DBus.Error.NoReply':
+            return True
+    return False
+
+def retryable(func):
+    """Call the function until its deferred succeed (max 5 times)."""
+
+    @defer.inlineCallbacks
+    def f(*a, **k):
+        """Built func."""
+        opportunities = 10
+        while opportunities:
+            try:
+                res = yield func(*a, **k)
+            except Exception, err:
+                opportunities -= 1
+                if opportunities == 0 or not _is_retry_exception(err):
+                    raise
+            else:
+                break
+        defer.returnValue(res)
+
+    return f
+
+
 class DBusInterface(object):
     """The DBus Interface to Ubuntu One's SyncDaemon."""
 
@@ -116,8 +145,9 @@
         return (name, description, is_error, is_connected,
                 is_online, queues, connection)
 
+    @retryable
     def get_status(self):
-        """Gets SD status."""
+        """Get SD status."""
         logger.info("Getting status")
         d = self.sync_daemon_tool.get_status()
         d.addCallback(self._process_status)
@@ -174,6 +204,7 @@
         logger.info("Received Share changed")
         self.msd.on_sd_shares_changed()
 
+    @retryable
     def get_content_queue(self):
         """Get the content queue from SDT."""
         def process(data):
@@ -237,6 +268,7 @@
 
         raise ValueError("Not supported MetaQueue data: %r" % data)
 
+    @retryable
     def get_meta_queue(self):
         """Get the meta queue from SDT."""
         def process(data):
@@ -254,6 +286,7 @@
         d.addCallback(process)
         return d
 
+    @retryable
     def get_folders(self):
         """Get the folders info from SDT."""
         def process(data):
@@ -309,7 +342,7 @@
         return started
 
     def _process_share_info(self, data):
-        """Processes share data."""
+        """Process share data."""
         all_items = []
         for d in data:
             logger.debug("    Share data: %r", d)
@@ -332,6 +365,7 @@
             all_items.append(s)
         return all_items
 
+    @retryable
     def get_shares_to_me(self):
         """Get the shares to me ('shares') info from SDT."""
         def process(data):
@@ -344,6 +378,7 @@
         d.addCallback(process)
         return d
 
+    @retryable
     def get_shares_to_others(self):
         """Get the shares to others ('shared') info from SDT."""
         def process(data):
@@ -355,4 +390,3 @@
         d = self.sync_daemon_tool.list_shared()
         d.addCallback(process)
         return d
-

=== modified file 'magicicada/syncdaemon.py'
--- magicicada/syncdaemon.py	2010-05-31 00:31:17 +0000
+++ magicicada/syncdaemon.py	2010-06-04 02:47:22 +0000
@@ -38,7 +38,7 @@
 
 
 class State(object):
-    """Holds the state of SD."""
+    """Hold the state of SD."""
     _attrs =  ['name', 'description', 'is_error', 'is_connected',
                'is_online', 'queues', 'connection', 'is_started']
 
@@ -61,7 +61,7 @@
             return self.__dict__[name]
 
     def _set(self, **data):
-        """Sets the attributes fromd data, if allowed."""
+        """Set the attributes from data, if allowed."""
         for name, value in data.iteritems():
             if name not in self._attrs:
                 raise AttributeError("Name not in _attrs: %r" % name)
@@ -129,7 +129,7 @@
 
     @defer.inlineCallbacks
     def _get_initial_data(self):
-        """Gets the initial SD data."""
+        """Get the initial SD data."""
         logger.info("Getting initial data")
 
         status_data = yield self.dbus.get_status()
@@ -259,22 +259,22 @@
                                                self._check_mq)
 
     def start(self):
-        """Starts the SyncDaemon."""
+        """Start the SyncDaemon."""
         logger.info("Starting u1.SD")
         self.dbus.start()
         self._get_initial_data()
 
     def quit(self):
-        """Stops the SyncDaemon and makes it quit."""
+        """Stop the SyncDaemon and makes it quit."""
         logger.info("Stopping u1.SD")
         self.dbus.quit()
 
     def connect(self):
-        """Tells the SyncDaemon that the user wants it to connect."""
+        """Tell the SyncDaemon that the user wants it to connect."""
         logger.info("Telling u1.SD to connect")
         self.dbus.connect()
 
     def disconnect(self):
-        """Tells the SyncDaemon that the user wants it to disconnect."""
+        """Tell the SyncDaemon that the user wants it to disconnect."""
         logger.info("Telling u1.SD to disconnect")
         self.dbus.disconnect()

=== modified file 'magicicada/tests/test_dbusiface.py'
--- magicicada/tests/test_dbusiface.py	2010-05-31 16:45:45 +0000
+++ magicicada/tests/test_dbusiface.py	2010-06-04 02:47:22 +0000
@@ -871,3 +871,110 @@
         self.assertTrue(self.handler.check_inf(
                         "Processing Shares To Others items (1)"))
         self.assertTrue(self.handler.check_dbg("    Share data: %r" % d))
+
+
+class RetryDecoratorTests(TwistedTestCase):
+    """Test the retry decorator."""
+
+    class Helper(object):
+        """Fails some times, finally succeeds."""
+        def __init__(self, limit, excep=None):
+            self.cant = 0
+            self.limit = limit
+            if excep is None:
+                self.excep = dbus.exceptions.DBusException(
+                                    name='org.freedesktop.DBus.Error.NoReply')
+            else:
+                self.excep = excep
+
+        def __call__(self):
+            """Called."""
+            self.cant += 1
+            if self.cant < self.limit:
+                return defer.fail(self.excep)
+            else:
+                return defer.succeed(True)
+
+    def test_retryexcep_noexcep(self):
+        """Test _is_retry_exception with no error."""
+        self.assertFalse(dbusiface._is_retry_exception("foo"))
+
+    def test_retryexcep_stdexcep(self):
+        """Test _is_retry_exception with a standard exception."""
+        self.assertFalse(dbusiface._is_retry_exception(NameError("foo")))
+
+    def test_retryexcep_dbusnoretry(self):
+        """Test _is_retry_exception with DBus exception, but not retry."""
+        err = dbus.exceptions.DBusException(name='org.freedesktop.DBus.Other')
+        self.assertFalse(dbusiface._is_retry_exception(err))
+
+    def test_retryexcep_dbusretry(self):
+        """Test _is_retry_exception with DBus exception, retry."""
+        err = dbus.exceptions.DBusException(
+                                    name='org.freedesktop.DBus.Error.NoReply')
+        self.assertTrue(dbusiface._is_retry_exception(err))
+
+    def get_decorated_func(self, func):
+        """Executes the test calling the received function."""
+
+        @dbusiface.retryable
+        def f():
+            """Test func."""
+            d = func()
+            return d
+
+        return f
+
+    def test_all_ok(self):
+        """All ok."""
+        f = self.get_decorated_func(lambda: defer.succeed(True))
+        d = f()
+        return d
+
+    def test_one_fail(self):
+        """One fail."""
+        deferred = defer.Deferred()
+        helper = self.Helper(2)
+        d = self.get_decorated_func(helper)()
+
+        def check(_):
+            """Check called quantity."""
+            self.assertEqual(helper.cant, 2)
+            deferred.callback(True)
+        d.addCallbacks(check,
+                       lambda _: deferred.errback(Exception()))
+        return deferred
+
+    def test_two_fails(self):
+        """Two fails."""
+        deferred = defer.Deferred()
+        helper = self.Helper(3)
+        d = self.get_decorated_func(helper)()
+
+        def check(_):
+            """Check called quantity."""
+            self.assertEqual(helper.cant, 3)
+            deferred.callback(True)
+        d.addCallbacks(check,
+                       lambda _: deferred.errback(Exception()))
+        return deferred
+
+    def test_too_many_fails(self):
+        """Check that retryal is not forever."""
+        deferred = defer.Deferred()
+        helper = self.Helper(12)
+        d = self.get_decorated_func(helper)()
+
+        d.addCallbacks(lambda _: deferred.errback(Exception()),
+                       lambda _: deferred.callback(True))
+        return deferred
+
+    def test_other_exception(self):
+        """Less fails than limit, but not retrying exception."""
+        deferred = defer.Deferred()
+        helper = self.Helper(2, excep=NameError("foo"))
+        d = self.get_decorated_func(helper)()
+
+        d.addCallbacks(lambda _: deferred.errback(Exception()),
+                       lambda _: deferred.callback(True))
+        return deferred