Merge lp:~jamesh/storm/bug-374909 into lp:storm

[James Henstridge]

[James Henstridge (jamesh)]

Produce DisconnectionError exceptions if we detect that the connection has been dropped after the fact.

This can happen if code is using the raw connection outside of the control of Storm, such as with the Django ORM integration, or some bits of legacy code in Launchpad.

The branch also links storm.exceptions.InterfaceError to the database module's InterfaceError, which was not being done previously.

[Thomas Herve (therve)]

+ def test_wb_catch_already_disconnected(self):
+ """If Storm detects connections that have already been disconnected.

The sentence looks incomplete.

+1!

review: Approve

[lp:~jamesh/storm/bug-374909]

308. By James Henstridge on 2009-07-01

Fix docstring problem pointed out in therve's review.

[James Henstridge (jamesh)]

> + def test_wb_catch_already_disconnected(self):
> + """If Storm detects connections that have already been disconnected.
>
> The sentence looks incomplete.

The "If" at the beginning shouldn't have been there. I've fixed that now.

[Stuart Bishop (stub)]

Looks good to me.

review: Approve

=== modified file 'storm/database.py'
--- storm/database.py	2009-05-08 17:27:18 +0000
+++ storm/database.py	2009-07-01 03:20:12 +0000
@@ -28,7 +28,8 @@
 from storm.expr import Expr, State, compile
 from storm.tracer import trace
 from storm.variables import Variable
-from storm.exceptions import ClosedError, DatabaseError, DisconnectionError
+from storm.exceptions import (
+    ClosedError, DatabaseError, DisconnectionError, Error)
 from storm.uri import URI
 import storm
 
@@ -273,7 +274,7 @@
 
         @return: The dbapi cursor object, as fetched from L{build_raw_cursor}.
         """
-        raw_cursor = self.build_raw_cursor()
+        raw_cursor = self._check_disconnect(self.build_raw_cursor)
         self._check_disconnect(
             trace, "connection_raw_execute", self, raw_cursor,
             statement, params or ())
@@ -326,7 +327,7 @@
         """Run the given function, checking for database disconnections."""
         try:
             return function(*args, **kwargs)
-        except DatabaseError, exc:
+        except Error, exc:
             if self.is_disconnection_error(exc):
                 self._state = STATE_DISCONNECTED
                 self._raw_connection = None

=== modified file 'storm/databases/postgres.py'
--- storm/databases/postgres.py	2008-08-18 19:44:51 +0000
+++ storm/databases/postgres.py	2009-07-01 03:20:12 +0000
@@ -36,7 +36,7 @@
 from storm.variables import Variable, ListVariable, RawStrVariable
 from storm.database import Database, Connection, Result
 from storm.exceptions import (
-    install_exceptions, DatabaseError, DatabaseModuleError,
+    install_exceptions, DatabaseError, DatabaseModuleError, InterfaceError,
     OperationalError, ProgrammingError, TimeoutError)
 from storm.tracer import TimeoutTracer
 
@@ -285,17 +285,20 @@
                 yield param
 
     def is_disconnection_error(self, exc):
-        if not isinstance(exc, (OperationalError, ProgrammingError)):
+        if not isinstance(exc, (InterfaceError, OperationalError,
+                                ProgrammingError)):
             return False
 
         # XXX: 2007-09-17 jamesh
-        # I have no idea why I am seeing the last exception message
-        # after upgrading to Gutsy.
+        # The last message is for the benefit of old versions of
+        # psycopg2 (prior to 2.0.7) which have a bug related to
+        # stripping the error severity from the message.
         msg = str(exc)
         return ("server closed the connection unexpectedly" in msg or
                 "could not connect to server" in msg or
                 "no connection to the server" in msg or
                 "connection not open" in msg or
+                "connection already closed" in msg or
                 "losed the connection unexpectedly" in msg)
 
 

=== modified file 'storm/exceptions.py'
--- storm/exceptions.py	2008-01-31 21:29:55 +0000
+++ storm/exceptions.py	2009-07-01 03:20:12 +0000
@@ -130,7 +130,7 @@
 def install_exceptions(module):
     for exception in (Error, Warning, DatabaseError, InternalError,
                       OperationalError, ProgrammingError, IntegrityError,
-                      DataError, NotSupportedError):
+                      DataError, NotSupportedError, InterfaceError):
         module_exception = getattr(module, exception.__name__, None)
         if module_exception is not None:
             module_exception.__bases__ += (exception,)

=== modified file 'tests/databases/base.py'
--- tests/databases/base.py	2008-10-10 05:44:35 +0000
+++ tests/databases/base.py	2009-07-01 03:20:12 +0000
@@ -33,7 +33,7 @@
 from storm.database import *
 from storm.event import EventSystem
 from storm.exceptions import (
-    DatabaseModuleError, DisconnectionError, OperationalError)
+    DatabaseError, DatabaseModuleError, DisconnectionError, OperationalError)
 
 from tests.databases.proxy import ProxyTCPServer
 from tests.helper import MakePath
@@ -515,6 +515,28 @@
         self.proxy.restart()
         self.assertRaises(DisconnectionError, self.connection.commit)
 
+    def test_wb_catch_already_disconnected(self):
+        """Storm detects connections that have already been disconnected.
+
+        If the connection is being used outside of Storm's control,
+        then it is possible that Storm won't see the disconnection.
+        It should be able to recover from this situation though.
+        """
+        result = self.connection.execute("SELECT 1")
+        self.assertTrue(result.get_one())
+        self.proxy.restart()
+        # Perform an action that should result in a disconnection.
+        try:
+            cursor = self.connection._raw_connection.cursor()
+            cursor.execute("SELECT 1")
+            cursor.fetchone()
+        except DatabaseError, exc:
+            self.assertTrue(self.connection.is_disconnection_error(exc))
+        else:
+            self.fail("Disconnection was not caught.")
+        self.assertRaises(DisconnectionError,
+                          self.connection.execute, "SELECT 1")
+
     def test_connection_stays_disconnected_in_transaction(self):
         """Test that connection does not immediately reconnect."""
         result = self.connection.execute("SELECT 1")