Code review comment for lp:~jamesh/storm/bug-854787

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

The object returned by DatabaseWrapper._cursor() ends up being:

 1. StormCursorWrapper wrapping
 2. CursorWrapper from django.db.backends.postgresql_psycopg2.base wrapping
 3. a psycopg2 cursor

We're checking the exceptions at layer (1), and the exceptions are being modified at (2). The is_disconnection_error() code is different for each database backend, so I wanted to avoid duplicating the logic.

As for IntegrityError, there is no special handling in this branch because it isn't related to disconnection handling. The Django CursorWrapper classes do map those exceptions to their own IntegrityError though (I guess they think that error is important enough not to squash down to DatabaseError). I agree that it'd be better if Django didn't modify the error objects and used a strategy closer to what Storm does. I'll file a bug report about this, but I do want something that works with current Django releases.

I have also updated the branch to fix a related bug in the transaction registration code (and adds some extra tests that should have been in there from the start), which I believe fixes the last known problems related to disconnection handling.

« Back to merge proposal