Storm

Merge lp:~cjwatson/storm/psycopg-2.5 into lp:storm

  1. psycopg-2.5
  2. Merge into trunk
Proposed by Colin Watson
Status: Merged
Approved by: Björn Tillenius
Approved revision: 478
Merged at revision: 478
Proposed branch: lp:~cjwatson/storm/psycopg-2.5
Merge into: lp:storm
Diff against target: 29 lines (+9/-3)
1 file modified
storm/exceptions.py (+9/-3)
To merge this branch: bzr merge lp:~cjwatson/storm/psycopg-2.5
Reviewer Review Type Date Requested Status
Björn Tillenius (community) Approve
Adam Collard (community) Approve
Review via email: mp+278330@code.launchpad.net

Commit message

Use ABCMeta for exception base class injection

This requires Python >= 2.6 (which is unlikely to be a problem these days!),
and adds compatibility with psycopg2 >= 2.5.

Description of the change

Use ABCMeta for exception base class injection.

This lets us be compatible with psycopg2 >= 2.5, where Error and subclasses are implemented in a C extension so can't have their __bases__ attribute patched.

It does introduce a potential obstacle to porting to Python 3 due to a Python bug noted in a comment. Other possible approaches include converting StormError et al into tuples (but that would be an API break if anyone was using them for any purpose other than isinstance/issubclass/exception-handling) or re-raising all database exceptions as Storm exceptions (rather intrusive and possibly error-prone). Since Storm hasn't yet been ported to Python 3 anyway, this doesn't seem like a showstopper, and I think using ABCMeta is a pragmatic approach for the time being.

To post a comment you must log in.
Revision history for this message
Adam Collard (adam-collard) wrote :

Looks good to me! +1

review: Approve
Revision history for this message
Björn Tillenius (bjornt) wrote :

Looks good to me as well, +1!

I've confirmed both Landscape and Storm tests pass with this branch and python-psycopg2 2.6.1-1build1.

There are some django tests that fail in Storm, but they fail in trunk as well for me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'storm/exceptions.py'
2--- storm/exceptions.py 2010-10-23 21:02:22 +0000
3+++ storm/exceptions.py 2015-11-23 15:14:11 +0000
4@@ -18,10 +18,12 @@
5 # You should have received a copy of the GNU Lesser General Public License
6 # along with this program. If not, see <http://www.gnu.org/licenses/>.
7 #
8+from abc import ABCMeta
9+import types
10
11
12 class StormError(Exception):
13- pass
14+ __metaclass__ = ABCMeta
15
16
17 class CompileError(StormError):
18@@ -141,5 +143,9 @@
19 OperationalError, ProgrammingError, IntegrityError,
20 DataError, NotSupportedError, InterfaceError):
21 module_exception = getattr(module, exception.__name__, None)
22- if module_exception is not None:
23- module_exception.__bases__ += (exception,)
24+ if (module_exception is not None and
25+ isinstance(module_exception, (type, types.ClassType))):
26+ # XXX This may need to be revisited when porting to Python 3 if
27+ # virtual subclasses are still ignored for exception handling
28+ # (https://bugs.python.org/issue12029).
29+ exception.register(module_exception)

Subscribers

People subscribed via source and target branches

to status/vote changes: