Merge lp:~cjwatson/storm/refactor-exception-wrapping into lp:storm
- refactor-exception-wrapping
- Merge into trunk
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 509 | ||||||||
Proposed branch: | lp:~cjwatson/storm/refactor-exception-wrapping | ||||||||
Merge into: | lp:storm | ||||||||
Diff against target: |
423 lines (+228/-31) 6 files modified
NEWS (+4/-0) storm/database.py (+132/-1) storm/databases/postgres.py (+28/-7) storm/databases/sqlite.py (+15/-9) storm/exceptions.py (+46/-14) tests/database.py (+3/-0) |
||||||||
To merge this branch: | bzr merge lp:~cjwatson/storm/refactor-exception-wrapping | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Simon Poirier (community) | Approve | ||
Review via email: mp+369319@code.launchpad.net |
Commit message
Wrap DB-API exceptions rather than injecting virtual base classes.
Description of the change
Re-raise DB-API exceptions wrapped in exception types that inherit from both StormError and the appropriate DB-API exception type, rather than injecting virtual base classes. This preserves existing exception handling in applications while also being a viable approach in Python 3.
Once I managed to get the right wrappers in place, this ended up being surprisingly simple, which makes me feel good about it (my first draft had many more explicit calls to wrap_exceptions and so felt much more fragile). The main difficulty was in creating just the right wrapper exception types to maintain API compatibility without making the code painfully repetitive, and I finally got that to fall into place today.
I'm running the full Launchpad test suite over this at the moment, though it looks clean so far and I've already found and fixed some problems via some of its most Storm-sensitive tests.
- 509. By Colin Watson
-
Make wrapper exceptions look slightly more like the DB-API originals.
- 510. By Colin Watson
-
Improve comment.
Colin Watson (cjwatson) wrote : | # |
Simon Poirier (simpoir) wrote : | # |
+1
I ran the full landscape test suite against this without any apparent regression.
- 511. By Colin Watson
-
Use superclass __setattr__ rather than changing __dict__ directly.
- 512. By Colin Watson
-
Use more neutral language.
Colin Watson (cjwatson) : | # |
Preview Diff
1 | === modified file 'NEWS' | |||
2 | --- NEWS 2019-06-05 11:41:07 +0000 | |||
3 | +++ NEWS 2019-07-02 15:55:56 +0000 | |||
4 | @@ -8,6 +8,10 @@ | |||
5 | 8 | package, which can apply patches "in parallel" against a set of stores. See | 8 | package, which can apply patches "in parallel" against a set of stores. See |
6 | 9 | the module docstring for more information. | 9 | the module docstring for more information. |
7 | 10 | - Added tox testing support. | 10 | - Added tox testing support. |
8 | 11 | - Re-raise DB-API exceptions wrapped in exception types that inherit from | ||
9 | 12 | both StormError and the appropriate DB-API exception type, rather than | ||
10 | 13 | injecting virtual base classes. This preserves existing exception handling | ||
11 | 14 | in applications while also being a viable approach in Python 3. | ||
12 | 11 | 15 | ||
13 | 12 | Bug fixes | 16 | Bug fixes |
14 | 13 | --------- | 17 | --------- |
15 | 14 | 18 | ||
16 | === modified file 'storm/database.py' | |||
17 | --- storm/database.py 2019-06-05 11:41:07 +0000 | |||
18 | +++ storm/database.py 2019-07-02 15:55:56 +0000 | |||
19 | @@ -27,6 +27,12 @@ | |||
20 | 27 | 27 | ||
21 | 28 | from __future__ import print_function | 28 | from __future__ import print_function |
22 | 29 | 29 | ||
23 | 30 | try: | ||
24 | 31 | from collections.abc import Callable | ||
25 | 32 | except ImportError: | ||
26 | 33 | from collections import Callable | ||
27 | 34 | from functools import wraps | ||
28 | 35 | |||
29 | 30 | from storm.expr import Expr, State, compile | 36 | from storm.expr import Expr, State, compile |
30 | 31 | # Circular import: imported at the end of the module. | 37 | # Circular import: imported at the end of the module. |
31 | 32 | # from storm.tracer import trace | 38 | # from storm.tracer import trace |
32 | @@ -34,7 +40,7 @@ | |||
33 | 34 | from storm.xid import Xid | 40 | from storm.xid import Xid |
34 | 35 | from storm.exceptions import ( | 41 | from storm.exceptions import ( |
35 | 36 | ClosedError, ConnectionBlockedError, DatabaseError, DisconnectionError, | 42 | ClosedError, ConnectionBlockedError, DatabaseError, DisconnectionError, |
37 | 37 | Error, ProgrammingError) | 43 | Error, ProgrammingError, wrap_exceptions) |
38 | 38 | from storm.uri import URI | 44 | from storm.uri import URI |
39 | 39 | import storm | 45 | import storm |
40 | 40 | 46 | ||
41 | @@ -163,6 +169,78 @@ | |||
42 | 163 | return row | 169 | return row |
43 | 164 | 170 | ||
44 | 165 | 171 | ||
45 | 172 | class CursorWrapper(object): | ||
46 | 173 | """A DB-API cursor, wrapping exceptions as StormError instances.""" | ||
47 | 174 | |||
48 | 175 | def __init__(self, cursor, database): | ||
49 | 176 | super(CursorWrapper, self).__setattr__('_cursor', cursor) | ||
50 | 177 | super(CursorWrapper, self).__setattr__('_database', database) | ||
51 | 178 | |||
52 | 179 | def __getattr__(self, name): | ||
53 | 180 | attr = getattr(self._cursor, name) | ||
54 | 181 | if isinstance(attr, Callable): | ||
55 | 182 | @wraps(attr) | ||
56 | 183 | def wrapper(*args, **kwargs): | ||
57 | 184 | with wrap_exceptions(self._database): | ||
58 | 185 | return attr(*args, **kwargs) | ||
59 | 186 | |||
60 | 187 | return wrapper | ||
61 | 188 | else: | ||
62 | 189 | return attr | ||
63 | 190 | |||
64 | 191 | def __setattr__(self, name, value): | ||
65 | 192 | return setattr(self._cursor, name, value) | ||
66 | 193 | |||
67 | 194 | def __iter__(self): | ||
68 | 195 | with wrap_exceptions(self._database): | ||
69 | 196 | for item in self._cursor: | ||
70 | 197 | yield item | ||
71 | 198 | |||
72 | 199 | def __enter__(self): | ||
73 | 200 | return self | ||
74 | 201 | |||
75 | 202 | def __exit__(self, type_, value, tb): | ||
76 | 203 | with wrap_exceptions(self._database): | ||
77 | 204 | self.close() | ||
78 | 205 | |||
79 | 206 | |||
80 | 207 | class ConnectionWrapper(object): | ||
81 | 208 | """A DB-API connection, wrapping exceptions as StormError instances.""" | ||
82 | 209 | |||
83 | 210 | def __init__(self, connection, database): | ||
84 | 211 | self.__dict__['_connection'] = connection | ||
85 | 212 | self.__dict__['_database'] = database | ||
86 | 213 | |||
87 | 214 | def __getattr__(self, name): | ||
88 | 215 | attr = getattr(self._connection, name) | ||
89 | 216 | if isinstance(attr, Callable): | ||
90 | 217 | @wraps(attr) | ||
91 | 218 | def wrapper(*args, **kwargs): | ||
92 | 219 | with wrap_exceptions(self._database): | ||
93 | 220 | return attr(*args, **kwargs) | ||
94 | 221 | |||
95 | 222 | return wrapper | ||
96 | 223 | else: | ||
97 | 224 | return attr | ||
98 | 225 | |||
99 | 226 | def __setattr__(self, name, value): | ||
100 | 227 | return setattr(self._connection, name, value) | ||
101 | 228 | |||
102 | 229 | def __enter__(self): | ||
103 | 230 | return self | ||
104 | 231 | |||
105 | 232 | def __exit__(self, type_, value, tb): | ||
106 | 233 | with wrap_exceptions(self._database): | ||
107 | 234 | if type_ is None and value is None and tb is None: | ||
108 | 235 | self.commit() | ||
109 | 236 | else: | ||
110 | 237 | self.rollback() | ||
111 | 238 | |||
112 | 239 | def cursor(self): | ||
113 | 240 | with wrap_exceptions(self._database): | ||
114 | 241 | return CursorWrapper(self._connection.cursor(), self._database) | ||
115 | 242 | |||
116 | 243 | |||
117 | 166 | class Connection(object): | 244 | class Connection(object): |
118 | 167 | """A connection to a database. | 245 | """A connection to a database. |
119 | 168 | 246 | ||
120 | @@ -483,6 +561,7 @@ | |||
121 | 483 | 561 | ||
122 | 484 | def __init__(self, uri=None): | 562 | def __init__(self, uri=None): |
123 | 485 | self._uri = uri | 563 | self._uri = uri |
124 | 564 | self._exception_types = {} | ||
125 | 486 | 565 | ||
126 | 487 | def get_uri(self): | 566 | def get_uri(self): |
127 | 488 | """Return the URI object this database was created with.""" | 567 | """Return the URI object this database was created with.""" |
128 | @@ -512,6 +591,58 @@ | |||
129 | 512 | """ | 591 | """ |
130 | 513 | raise NotImplementedError | 592 | raise NotImplementedError |
131 | 514 | 593 | ||
132 | 594 | @property | ||
133 | 595 | def _exception_module(self): | ||
134 | 596 | """The module where appropriate DB-API exception types are defined. | ||
135 | 597 | |||
136 | 598 | Subclasses should set this if they support re-raising DB-API | ||
137 | 599 | exceptions as StormError instances. | ||
138 | 600 | """ | ||
139 | 601 | return None | ||
140 | 602 | |||
141 | 603 | def _make_combined_exception_type(self, wrapper_type, dbapi_type): | ||
142 | 604 | """Make a combined exception based on both DB-API and Storm. | ||
143 | 605 | |||
144 | 606 | Storm historically defined its own exception types as ABCs and | ||
145 | 607 | registered the DB-API exception types as virtual subclasses. | ||
146 | 608 | However, this doesn't work properly in Python 3 | ||
147 | 609 | (https://bugs.python.org/issue12029). Instead, we create and cache | ||
148 | 610 | subclass-specific exception types that inherit from both StormError | ||
149 | 611 | and the DB-API exception type, allowing code that catches either | ||
150 | 612 | StormError (or subclasses) or the specific DB-API exceptions to keep | ||
151 | 613 | working. | ||
152 | 614 | |||
153 | 615 | @type wrapper_type: L{type} | ||
154 | 616 | @param wrapper_type: The type of the wrapper exception to create; a | ||
155 | 617 | subclass of L{StormError}. | ||
156 | 618 | @type dbapi_type: L{type} | ||
157 | 619 | @param dbapi_type: The type of the DB-API exception. | ||
158 | 620 | |||
159 | 621 | @return: The combined exception type. | ||
160 | 622 | """ | ||
161 | 623 | if wrapper_type not in self._exception_types: | ||
162 | 624 | self._exception_types[wrapper_type] = type( | ||
163 | 625 | dbapi_type.__name__, (dbapi_type, wrapper_type), {}) | ||
164 | 626 | return self._exception_types[wrapper_type] | ||
165 | 627 | |||
166 | 628 | def _wrap_exception(self, wrapper_type, exception): | ||
167 | 629 | """Wrap a DB-API exception as a StormError instance. | ||
168 | 630 | |||
169 | 631 | This constructs a wrapper exception with the same C{args} as the | ||
170 | 632 | DB-API exception. Subclasses may override this to set additional | ||
171 | 633 | attributes on the wrapper exception. | ||
172 | 634 | |||
173 | 635 | @type wrapper_type: L{type} | ||
174 | 636 | @param wrapper_type: The type of the wrapper exception to create; a | ||
175 | 637 | subclass of L{StormError}. | ||
176 | 638 | @type exception: L{Exception} | ||
177 | 639 | @param exception: The DB-API exception to wrap. | ||
178 | 640 | |||
179 | 641 | @return: The wrapped exception; an instance of L{StormError}. | ||
180 | 642 | """ | ||
181 | 643 | return self._make_combined_exception_type( | ||
182 | 644 | wrapper_type, exception.__class__)(*exception.args) | ||
183 | 645 | |||
184 | 515 | 646 | ||
185 | 516 | def convert_param_marks(statement, from_param_mark, to_param_mark): | 647 | def convert_param_marks(statement, from_param_mark, to_param_mark): |
186 | 517 | # TODO: Add support for $foo$bar$foo$ literals. | 648 | # TODO: Add support for $foo$bar$foo$ literals. |
187 | 518 | 649 | ||
188 | === modified file 'storm/databases/postgres.py' | |||
189 | --- storm/databases/postgres.py 2019-06-05 11:41:07 +0000 | |||
190 | +++ storm/databases/postgres.py 2019-07-02 15:55:56 +0000 | |||
191 | @@ -50,14 +50,13 @@ | |||
192 | 50 | from storm.variables import ( | 50 | from storm.variables import ( |
193 | 51 | Variable, ListVariable, JSONVariable as BaseJSONVariable) | 51 | Variable, ListVariable, JSONVariable as BaseJSONVariable) |
194 | 52 | from storm.properties import SimpleProperty | 52 | from storm.properties import SimpleProperty |
196 | 53 | from storm.database import Database, Connection, Result | 53 | from storm.database import Database, Connection, ConnectionWrapper, Result |
197 | 54 | from storm.exceptions import ( | 54 | from storm.exceptions import ( |
200 | 55 | install_exceptions, DatabaseError, DatabaseModuleError, InterfaceError, | 55 | DatabaseError, DatabaseModuleError, InterfaceError, |
201 | 56 | OperationalError, ProgrammingError, TimeoutError, Error) | 56 | OperationalError, ProgrammingError, TimeoutError, Error, wrap_exceptions) |
202 | 57 | from storm.tracer import TimeoutTracer | 57 | from storm.tracer import TimeoutTracer |
203 | 58 | 58 | ||
204 | 59 | 59 | ||
205 | 60 | install_exceptions(psycopg2) | ||
206 | 61 | compile = compile.create_child() | 60 | compile = compile.create_child() |
207 | 62 | 61 | ||
208 | 63 | 62 | ||
209 | @@ -366,6 +365,8 @@ | |||
210 | 366 | 365 | ||
211 | 367 | connection_factory = PostgresConnection | 366 | connection_factory = PostgresConnection |
212 | 368 | 367 | ||
213 | 368 | _exception_module = psycopg2 | ||
214 | 369 | |||
215 | 369 | # An integer representing the server version. If the server does | 370 | # An integer representing the server version. If the server does |
216 | 370 | # not support the server_version_num variable, this will be set to | 371 | # not support the server_version_num variable, this will be set to |
217 | 371 | # 0. In practice, this means the variable will be 0 or greater | 372 | # 0. In practice, this means the variable will be 0 or greater |
218 | @@ -397,15 +398,31 @@ | |||
219 | 397 | "'autocommit', 'serializable', 'read-committed'" % | 398 | "'autocommit', 'serializable', 'read-committed'" % |
220 | 398 | (isolation,)) | 399 | (isolation,)) |
221 | 399 | 400 | ||
224 | 400 | def raw_connect(self): | 401 | _psycopg_error_attributes = ("pgerror", "pgcode", "cursor", "diag") |
225 | 401 | raw_connection = psycopg2.connect(self._dsn) | 402 | |
226 | 403 | def _make_combined_exception_type(self, wrapper_type, dbapi_type): | ||
227 | 404 | combined_type = super(Postgres, self)._make_combined_exception_type( | ||
228 | 405 | wrapper_type, dbapi_type) | ||
229 | 406 | for name in self._psycopg_error_attributes: | ||
230 | 407 | setattr(combined_type, name, lambda err: getattr(err, "_" + name)) | ||
231 | 408 | return combined_type | ||
232 | 409 | |||
233 | 410 | def _wrap_exception(self, wrapper_type, exception): | ||
234 | 411 | wrapped = super(Postgres, self)._wrap_exception( | ||
235 | 412 | wrapper_type, exception) | ||
236 | 413 | for name in self._psycopg_error_attributes: | ||
237 | 414 | setattr(wrapped, "_" + name, getattr(exception, name)) | ||
238 | 415 | return wrapped | ||
239 | 416 | |||
240 | 417 | def _raw_connect(self): | ||
241 | 418 | raw_connection = ConnectionWrapper(psycopg2.connect(self._dsn), self) | ||
242 | 402 | 419 | ||
243 | 403 | if self._version is None: | 420 | if self._version is None: |
244 | 404 | cursor = raw_connection.cursor() | 421 | cursor = raw_connection.cursor() |
245 | 405 | 422 | ||
246 | 406 | try: | 423 | try: |
247 | 407 | cursor.execute("SHOW server_version_num") | 424 | cursor.execute("SHOW server_version_num") |
249 | 408 | except psycopg2.ProgrammingError: | 425 | except ProgrammingError: |
250 | 409 | self._version = 0 | 426 | self._version = 0 |
251 | 410 | else: | 427 | else: |
252 | 411 | self._version = int(cursor.fetchone()[0]) | 428 | self._version = int(cursor.fetchone()[0]) |
253 | @@ -415,6 +432,10 @@ | |||
254 | 415 | raw_connection.set_isolation_level(self._isolation) | 432 | raw_connection.set_isolation_level(self._isolation) |
255 | 416 | return raw_connection | 433 | return raw_connection |
256 | 417 | 434 | ||
257 | 435 | def raw_connect(self): | ||
258 | 436 | with wrap_exceptions(self): | ||
259 | 437 | return self._raw_connect() | ||
260 | 438 | |||
261 | 418 | 439 | ||
262 | 419 | create_from_uri = Postgres | 440 | create_from_uri = Postgres |
263 | 420 | 441 | ||
264 | 421 | 442 | ||
265 | === modified file 'storm/databases/sqlite.py' | |||
266 | --- storm/databases/sqlite.py 2019-06-07 12:17:35 +0000 | |||
267 | +++ storm/databases/sqlite.py 2019-07-02 15:55:56 +0000 | |||
268 | @@ -35,16 +35,14 @@ | |||
269 | 35 | sqlite = dummy | 35 | sqlite = dummy |
270 | 36 | 36 | ||
271 | 37 | from storm.variables import Variable, RawStrVariable | 37 | from storm.variables import Variable, RawStrVariable |
274 | 38 | from storm.database import Database, Connection, Result | 38 | from storm.database import Database, Connection, ConnectionWrapper, Result |
275 | 39 | from storm.exceptions import install_exceptions, DatabaseModuleError | 39 | from storm.exceptions import ( |
276 | 40 | DatabaseModuleError, OperationalError, wrap_exceptions) | ||
277 | 40 | from storm.expr import ( | 41 | from storm.expr import ( |
278 | 41 | Insert, Select, SELECT, Undef, SQLRaw, Union, Except, Intersect, | 42 | Insert, Select, SELECT, Undef, SQLRaw, Union, Except, Intersect, |
279 | 42 | compile, compile_insert, compile_select) | 43 | compile, compile_insert, compile_select) |
280 | 43 | 44 | ||
281 | 44 | 45 | ||
282 | 45 | install_exceptions(sqlite) | ||
283 | 46 | |||
284 | 47 | |||
285 | 48 | compile = compile.create_child() | 46 | compile = compile.create_child() |
286 | 49 | 47 | ||
287 | 50 | @compile.when(Select) | 48 | @compile.when(Select) |
288 | @@ -160,7 +158,7 @@ | |||
289 | 160 | while True: | 158 | while True: |
290 | 161 | try: | 159 | try: |
291 | 162 | return Connection.raw_execute(self, statement, params) | 160 | return Connection.raw_execute(self, statement, params) |
293 | 163 | except sqlite.OperationalError as e: | 161 | except OperationalError as e: |
294 | 164 | if str(e) != "database is locked": | 162 | if str(e) != "database is locked": |
295 | 165 | raise | 163 | raise |
296 | 166 | elif now() - started < self._database._timeout: | 164 | elif now() - started < self._database._timeout: |
297 | @@ -180,6 +178,8 @@ | |||
298 | 180 | 178 | ||
299 | 181 | connection_factory = SQLiteConnection | 179 | connection_factory = SQLiteConnection |
300 | 182 | 180 | ||
301 | 181 | _exception_module = sqlite | ||
302 | 182 | |||
303 | 183 | def __init__(self, uri): | 183 | def __init__(self, uri): |
304 | 184 | super(SQLite, self).__init__(uri) | 184 | super(SQLite, self).__init__(uri) |
305 | 185 | if sqlite is dummy: | 185 | if sqlite is dummy: |
306 | @@ -190,10 +190,12 @@ | |||
307 | 190 | self._journal_mode = uri.options.get("journal_mode") | 190 | self._journal_mode = uri.options.get("journal_mode") |
308 | 191 | self._foreign_keys = uri.options.get("foreign_keys") | 191 | self._foreign_keys = uri.options.get("foreign_keys") |
309 | 192 | 192 | ||
311 | 193 | def raw_connect(self): | 193 | def _raw_connect(self): |
312 | 194 | # See the story at the end to understand why we set isolation_level. | 194 | # See the story at the end to understand why we set isolation_level. |
315 | 195 | raw_connection = sqlite.connect(self._filename, timeout=self._timeout, | 195 | raw_connection = ConnectionWrapper( |
316 | 196 | isolation_level=None) | 196 | sqlite.connect( |
317 | 197 | self._filename, timeout=self._timeout, isolation_level=None), | ||
318 | 198 | self) | ||
319 | 197 | if self._synchronous is not None: | 199 | if self._synchronous is not None: |
320 | 198 | raw_connection.execute("PRAGMA synchronous = %s" % | 200 | raw_connection.execute("PRAGMA synchronous = %s" % |
321 | 199 | (self._synchronous,)) | 201 | (self._synchronous,)) |
322 | @@ -208,6 +210,10 @@ | |||
323 | 208 | 210 | ||
324 | 209 | return raw_connection | 211 | return raw_connection |
325 | 210 | 212 | ||
326 | 213 | def raw_connect(self): | ||
327 | 214 | with wrap_exceptions(self): | ||
328 | 215 | return self._raw_connect() | ||
329 | 216 | |||
330 | 211 | 217 | ||
331 | 212 | create_from_uri = SQLite | 218 | create_from_uri = SQLite |
332 | 213 | 219 | ||
333 | 214 | 220 | ||
334 | === modified file 'storm/exceptions.py' | |||
335 | --- storm/exceptions.py 2019-06-07 16:35:06 +0000 | |||
336 | +++ storm/exceptions.py 2019-07-02 15:55:56 +0000 | |||
337 | @@ -20,12 +20,14 @@ | |||
338 | 20 | # | 20 | # |
339 | 21 | from __future__ import print_function | 21 | from __future__ import print_function |
340 | 22 | 22 | ||
343 | 23 | from abc import ABCMeta | 23 | from contextlib import contextmanager |
344 | 24 | import types | 24 | import sys |
345 | 25 | |||
346 | 26 | import six | ||
347 | 25 | 27 | ||
348 | 26 | 28 | ||
349 | 27 | class StormError(Exception): | 29 | class StormError(Exception): |
351 | 28 | __metaclass__ = ABCMeta | 30 | pass |
352 | 29 | 31 | ||
353 | 30 | 32 | ||
354 | 31 | class CompileError(StormError): | 33 | class CompileError(StormError): |
355 | @@ -140,14 +142,44 @@ | |||
356 | 140 | """Raised when an attempt is made to use a blocked connection.""" | 142 | """Raised when an attempt is made to use a blocked connection.""" |
357 | 141 | 143 | ||
358 | 142 | 144 | ||
370 | 143 | def install_exceptions(module): | 145 | # More generic exceptions must come later. For convenience, use the order |
371 | 144 | for exception in (Error, Warning, DatabaseError, InternalError, | 146 | # of definition above and then reverse it. |
372 | 145 | OperationalError, ProgrammingError, IntegrityError, | 147 | _wrapped_exception_types = tuple(reversed(( |
373 | 146 | DataError, NotSupportedError, InterfaceError): | 148 | Error, |
374 | 147 | module_exception = getattr(module, exception.__name__, None) | 149 | Warning, |
375 | 148 | if (module_exception is not None and | 150 | InterfaceError, |
376 | 149 | isinstance(module_exception, (type, types.ClassType))): | 151 | DatabaseError, |
377 | 150 | # XXX This may need to be revisited when porting to Python 3 if | 152 | InternalError, |
378 | 151 | # virtual subclasses are still ignored for exception handling | 153 | OperationalError, |
379 | 152 | # (https://bugs.python.org/issue12029). | 154 | ProgrammingError, |
380 | 153 | exception.register(module_exception) | 155 | IntegrityError, |
381 | 156 | DataError, | ||
382 | 157 | NotSupportedError, | ||
383 | 158 | ))) | ||
384 | 159 | |||
385 | 160 | |||
386 | 161 | @contextmanager | ||
387 | 162 | def wrap_exceptions(database): | ||
388 | 163 | """Context manager that re-raises DB exceptions as StormError instances.""" | ||
389 | 164 | try: | ||
390 | 165 | yield | ||
391 | 166 | except Exception as e: | ||
392 | 167 | module = database._exception_module | ||
393 | 168 | if module is None: | ||
394 | 169 | # This backend does not support re-raising database exceptions. | ||
395 | 170 | raise | ||
396 | 171 | for wrapper_type in _wrapped_exception_types: | ||
397 | 172 | dbapi_type = getattr(module, wrapper_type.__name__, None) | ||
398 | 173 | if (dbapi_type is not None and | ||
399 | 174 | isinstance(dbapi_type, six.class_types) and | ||
400 | 175 | isinstance(e, dbapi_type)): | ||
401 | 176 | wrapped = database._wrap_exception(wrapper_type, e) | ||
402 | 177 | tb = sys.exc_info()[2] | ||
403 | 178 | # As close to "raise wrapped.with_traceback(tb) from e" as | ||
404 | 179 | # we can manage, but without causing syntax errors on | ||
405 | 180 | # various versions of Python. | ||
406 | 181 | if six.PY2: | ||
407 | 182 | six.reraise(wrapped, None, tb) | ||
408 | 183 | else: | ||
409 | 184 | six.raise_from(wrapped.with_traceback(tb), e) | ||
410 | 185 | raise | ||
411 | 154 | 186 | ||
412 | === modified file 'tests/database.py' | |||
413 | --- tests/database.py 2019-06-05 11:41:07 +0000 | |||
414 | +++ tests/database.py 2019-07-02 15:55:56 +0000 | |||
415 | @@ -96,6 +96,9 @@ | |||
416 | 96 | 96 | ||
417 | 97 | class FakeConnection(object): | 97 | class FakeConnection(object): |
418 | 98 | 98 | ||
419 | 99 | def __init__(self): | ||
420 | 100 | self._database = Database() | ||
421 | 101 | |||
422 | 99 | def _check_disconnect(self, _function, *args, **kwargs): | 102 | def _check_disconnect(self, _function, *args, **kwargs): |
423 | 100 | return _function(*args, **kwargs) | 103 | return _function(*args, **kwargs) |
424 | 101 | 104 |
I've done a full successful Launchpad test run with this patch. The only change I had to make to Launchpad was to adjust LaunchpadDataba se.__init_ _, which is just because it's subclassing Postgres while (for unrelated reasons) not running the superclass constructor, so as a consequence it has to keep up with changes to the superclass constructor, which I think is fine.