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

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

You asked for suggestions too, so...

The disconnection errors don't change. Should the connection object have a method to register extra disconnection errors instead of them being passed in as arguments?

Instead of providing a list of exceptions and relying on the existing behavior of is_disconnection_error, should a callable be passed instead? This would provide more control to sniff the database exception when it isn't generic enough. Or is the goal here to insure the current is_disconnection_error logic is used because the PG backend already sniffs the errors strings?

How does this code handle and integrity violation (or other DatabaseError) btw? Given these seem to be thunked down from IntegrityError to DatabaseError it would be worth a test even if the test is demonstrating broken behavior and citing a bug in the Django bug tracker.

« Back to merge proposal