Opened 12 years ago
Closed 12 years ago
#19920 closed Bug (fixed)
DatabaseError exceptions in _commit, _rollback, etc.
Reported by: | Aymeric Augustin | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
#14223 unified the IntegrityError exceptions raised in _commit.
For consistency, Django should also unify DatabaseError exceptions raised in _commit and _rollback.
To be fully consistent, Django should also wrap exceptions in savepoint-related methods.
I'm going to need this for error handling in persistent connections.
With the current approach this is going to result in a lot of duplicated code:
try: # do stuff except Database.IntegrityError as e: six.reraise(utils.IntegrityError, utils.IntegrityError(*tuple(e.args)), sys.exc_info()[2]) except Database.DatabaseError as e: six.reraise(utils.DatabaseError, utils.DatabaseError(*tuple(e.args)), sys.exc_info()[2])
A better solution would be to abstract this pattern into a decorator.
A less magical alternative is to make the exceptions attributes of the database wrapper. This would allow code such as:
for conn in connections.all(): try: conn.rollback() except conn.DatabaseError: pass
This solution seems more robust to me.
It's suggested as an extension by PEP249: http://www.python.org/dev/peps/pep-0249/#connection-error
It's more backwards compatible with the current code because it doesn't require changing the type of some exceptions.
It avoids problems with catching the "wrong" DatabaseError (Django's vs. the database adapter's). This is an easy mistake, and database exceptions are very hard to test — one cannot shut down the database during tests!
I'm mostly logging this alternative for posterity, because Django made a different choice long ago, and reversing it would be complicated — all the more since a non PEP-249 exception was introduced in the hierarchy: ProtectedError, see https://docs.djangoproject.com/en/dev/ref/exceptions/#transaction-exceptions
Change History (2)
comment:1 by , 12 years ago
Has patch: | set |
---|
comment:2 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
https://github.com/django/django/pull/855