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

Proposed by James Henstridge
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~jamesh/storm/bug-374909
Merge into: lp://staging/storm
Diff against target: 123 lines
To merge this branch: bzr merge lp://staging/~jamesh/storm/bug-374909
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Thomas Herve (community) Approve
Review via email: mp+6606@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

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.

Revision history for this message
Thomas Herve (therve) wrote :

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

The sentence looks incomplete.

+1!

review: Approve
308. By James Henstridge

Fix docstring problem pointed out in therve's review.

Revision history for this message
James Henstridge (jamesh) wrote :

> + 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.

Revision history for this message
Stuart Bishop (stub) wrote :

Looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
28from storm.expr import Expr, State, compile28from storm.expr import Expr, State, compile
29from storm.tracer import trace29from storm.tracer import trace
30from storm.variables import Variable30from storm.variables import Variable
31from storm.exceptions import ClosedError, DatabaseError, DisconnectionError31from storm.exceptions import (
32 ClosedError, DatabaseError, DisconnectionError, Error)
32from storm.uri import URI33from storm.uri import URI
33import storm34import storm
3435
@@ -273,7 +274,7 @@
273274
274 @return: The dbapi cursor object, as fetched from L{build_raw_cursor}.275 @return: The dbapi cursor object, as fetched from L{build_raw_cursor}.
275 """276 """
276 raw_cursor = self.build_raw_cursor()277 raw_cursor = self._check_disconnect(self.build_raw_cursor)
277 self._check_disconnect(278 self._check_disconnect(
278 trace, "connection_raw_execute", self, raw_cursor,279 trace, "connection_raw_execute", self, raw_cursor,
279 statement, params or ())280 statement, params or ())
@@ -326,7 +327,7 @@
326 """Run the given function, checking for database disconnections."""327 """Run the given function, checking for database disconnections."""
327 try:328 try:
328 return function(*args, **kwargs)329 return function(*args, **kwargs)
329 except DatabaseError, exc:330 except Error, exc:
330 if self.is_disconnection_error(exc):331 if self.is_disconnection_error(exc):
331 self._state = STATE_DISCONNECTED332 self._state = STATE_DISCONNECTED
332 self._raw_connection = None333 self._raw_connection = None
333334
=== 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 @@
36from storm.variables import Variable, ListVariable, RawStrVariable36from storm.variables import Variable, ListVariable, RawStrVariable
37from storm.database import Database, Connection, Result37from storm.database import Database, Connection, Result
38from storm.exceptions import (38from storm.exceptions import (
39 install_exceptions, DatabaseError, DatabaseModuleError,39 install_exceptions, DatabaseError, DatabaseModuleError, InterfaceError,
40 OperationalError, ProgrammingError, TimeoutError)40 OperationalError, ProgrammingError, TimeoutError)
41from storm.tracer import TimeoutTracer41from storm.tracer import TimeoutTracer
4242
@@ -285,17 +285,20 @@
285 yield param285 yield param
286286
287 def is_disconnection_error(self, exc):287 def is_disconnection_error(self, exc):
288 if not isinstance(exc, (OperationalError, ProgrammingError)):288 if not isinstance(exc, (InterfaceError, OperationalError,
289 ProgrammingError)):
289 return False290 return False
290291
291 # XXX: 2007-09-17 jamesh292 # XXX: 2007-09-17 jamesh
292 # I have no idea why I am seeing the last exception message293 # The last message is for the benefit of old versions of
293 # after upgrading to Gutsy.294 # psycopg2 (prior to 2.0.7) which have a bug related to
295 # stripping the error severity from the message.
294 msg = str(exc)296 msg = str(exc)
295 return ("server closed the connection unexpectedly" in msg or297 return ("server closed the connection unexpectedly" in msg or
296 "could not connect to server" in msg or298 "could not connect to server" in msg or
297 "no connection to the server" in msg or299 "no connection to the server" in msg or
298 "connection not open" in msg or300 "connection not open" in msg or
301 "connection already closed" in msg or
299 "losed the connection unexpectedly" in msg)302 "losed the connection unexpectedly" in msg)
300303
301304
302305
=== 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 @@
130def install_exceptions(module):130def install_exceptions(module):
131 for exception in (Error, Warning, DatabaseError, InternalError,131 for exception in (Error, Warning, DatabaseError, InternalError,
132 OperationalError, ProgrammingError, IntegrityError,132 OperationalError, ProgrammingError, IntegrityError,
133 DataError, NotSupportedError):133 DataError, NotSupportedError, InterfaceError):
134 module_exception = getattr(module, exception.__name__, None)134 module_exception = getattr(module, exception.__name__, None)
135 if module_exception is not None:135 if module_exception is not None:
136 module_exception.__bases__ += (exception,)136 module_exception.__bases__ += (exception,)
137137
=== 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 @@
33from storm.database import *33from storm.database import *
34from storm.event import EventSystem34from storm.event import EventSystem
35from storm.exceptions import (35from storm.exceptions import (
36 DatabaseModuleError, DisconnectionError, OperationalError)36 DatabaseError, DatabaseModuleError, DisconnectionError, OperationalError)
3737
38from tests.databases.proxy import ProxyTCPServer38from tests.databases.proxy import ProxyTCPServer
39from tests.helper import MakePath39from tests.helper import MakePath
@@ -515,6 +515,28 @@
515 self.proxy.restart()515 self.proxy.restart()
516 self.assertRaises(DisconnectionError, self.connection.commit)516 self.assertRaises(DisconnectionError, self.connection.commit)
517517
518 def test_wb_catch_already_disconnected(self):
519 """Storm detects connections that have already been disconnected.
520
521 If the connection is being used outside of Storm's control,
522 then it is possible that Storm won't see the disconnection.
523 It should be able to recover from this situation though.
524 """
525 result = self.connection.execute("SELECT 1")
526 self.assertTrue(result.get_one())
527 self.proxy.restart()
528 # Perform an action that should result in a disconnection.
529 try:
530 cursor = self.connection._raw_connection.cursor()
531 cursor.execute("SELECT 1")
532 cursor.fetchone()
533 except DatabaseError, exc:
534 self.assertTrue(self.connection.is_disconnection_error(exc))
535 else:
536 self.fail("Disconnection was not caught.")
537 self.assertRaises(DisconnectionError,
538 self.connection.execute, "SELECT 1")
539
518 def test_connection_stays_disconnected_in_transaction(self):540 def test_connection_stays_disconnected_in_transaction(self):
519 """Test that connection does not immediately reconnect."""541 """Test that connection does not immediately reconnect."""
520 result = self.connection.execute("SELECT 1")542 result = self.connection.execute("SELECT 1")

Subscribers

People subscribed via source and target branches

to status/vote changes: