diff options
| author | Mike Bayer <mike_mp@zzzcomputing.com> | 2010-09-15 22:20:01 -0400 |
|---|---|---|
| committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2010-09-15 22:20:01 -0400 |
| commit | 52735e75c527dd24a793ae7ee0b1978e8aacacb5 (patch) | |
| tree | f70345fd8a325517df81d8e057d5c51078cd9d0a | |
| parent | e79158e296a8f889e823cf5c7ce89747ed242eb4 (diff) | |
| download | sqlalchemy-52735e75c527dd24a793ae7ee0b1978e8aacacb5.tar.gz | |
- fix test_single test to use default dialect
- The exception raised by Session when it is used
subsequent to a subtransaction rollback (which is what
happens when a flush fails in autocommit=False mode) has
now been reworded (this is the "inactive due to a
rollback in a subtransaction" message). In particular,
if the rollback was due to an exception during flush(),
the message states this is the case, and reiterates the
string form of the original exception that occurred
during flush. If the session is closed due to explicit
usage of subtransactions (not very common), the message
just states this is the case.
- The exception raised by Mapper when repeated requests to
its initialization are made after initialization already
failed no longer assumes the "hasattr" case, since
there's other scenarios in which this message gets
emitted, and the message also does not compound onto
itself multiple times - you get the same message for
each attempt at usage. The misnomer "compiles" is being
traded out for "initialize".
| -rw-r--r-- | CHANGES | 31 | ||||
| -rwxr-xr-x | lib/sqlalchemy/ext/declarative.py | 2 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/dependency.py | 6 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 18 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 38 | ||||
| -rw-r--r-- | test/ext/test_declarative.py | 13 | ||||
| -rw-r--r-- | test/orm/inheritance/test_single.py | 6 | ||||
| -rw-r--r-- | test/orm/test_mapper.py | 23 | ||||
| -rw-r--r-- | test/orm/test_session.py | 33 |
9 files changed, 130 insertions, 40 deletions
@@ -15,6 +15,27 @@ CHANGES - Fixed a regression in 0.6.4 which occurred if you passed an empty list to "include_properties" on mapper() [ticket:1918] + + - The exception raised by Session when it is used + subsequent to a subtransaction rollback (which is what + happens when a flush fails in autocommit=False mode) has + now been reworded (this is the "inactive due to a + rollback in a subtransaction" message). In particular, + if the rollback was due to an exception during flush(), + the message states this is the case, and reiterates the + string form of the original exception that occurred + during flush. If the session is closed due to explicit + usage of subtransactions (not very common), the message + just states this is the case. + + - The exception raised by Mapper when repeated requests to + its initialization are made after initialization already + failed no longer assumes the "hasattr" case, since + there's other scenarios in which this message gets + emitted, and the message also does not compound onto + itself multiple times - you get the same message for + each attempt at usage. The misnomer "compiles" is being + traded out for "initialize". - Added an assertion during flush which ensures that no NULL-holding identity keys were generated @@ -42,11 +63,11 @@ CHANGES after a flush. The flag is only intended for very specific use cases. - - Slight improvement to the behavior of "passive_updates=False" - when placed only on the many-to-one side of a - relationship; documentation has been clarified - that passive_updates=False should really be on the - one-to-many side. + - Slight improvement to the behavior of + "passive_updates=False" when placed only on the + many-to-one side of a relationship; documentation has + been clarified that passive_updates=False should really + be on the one-to-many side. - Placing passive_deletes=True on a many-to-one emits a warning, since you probably intended to put it on diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py index 6d4bdda43..40263c7b8 100755 --- a/lib/sqlalchemy/ext/declarative.py +++ b/lib/sqlalchemy/ext/declarative.py @@ -1192,7 +1192,7 @@ def _deferred_relationship(cls, prop): return x except NameError, n: raise exceptions.InvalidRequestError( - "When compiling mapper %s, expression %r failed to " + "When initializing mapper %s, expression %r failed to " "locate a name (%r). If this is a class name, consider " "adding this relationship() to the %r class after " "both dependent classes have been defined." % diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 662cfc67b..4458a8547 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -806,8 +806,10 @@ class DetectKeySwitch(DependencyProcessor): if not issubclass(state.class_, self.parent.class_): continue dict_ = state.dict - related = state.get_impl(self.key).get(state, dict_, passive=self.passive_updates) - if related is not attributes.PASSIVE_NO_RESULT and related is not None: + related = state.get_impl(self.key).get(state, dict_, + passive=self.passive_updates) + if related is not attributes.PASSIVE_NO_RESULT and \ + related is not None: related_state = attributes.instance_state(dict_[self.key]) if related_state in switchers: uowcommit.register_object(state, diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 8d31dd89a..49f5d2190 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -30,6 +30,7 @@ from sqlalchemy.orm.util import ( ExtensionCarrier, _INSTRUMENTOR, _class_to_mapper, _state_mapper, class_mapper, instance_str, state_str, ) +import sys __all__ = ( 'Mapper', @@ -787,12 +788,13 @@ class Mapper(object): # the order of mapper compilation for mapper in list(_mapper_registry): if getattr(mapper, '_compile_failed', False): - raise sa_exc.InvalidRequestError( - "One or more mappers failed to compile. " - "Exception was probably " - "suppressed within a hasattr() call. " - "Message was: %s" % - mapper._compile_failed) + e = sa_exc.InvalidRequestError( + "One or more mappers failed to initialize - " + "can't proceed with initialization of other " + "mappers. Original exception was: %s" + % mapper._compile_failed) + e._compile_failed = mapper._compile_failed + raise e if not mapper.compiled: mapper._post_configure_properties() @@ -801,9 +803,9 @@ class Mapper(object): finally: _already_compiling = False except: - import sys exc = sys.exc_info()[1] - self._compile_failed = exc + if not hasattr(exc, '_compile_failed'): + self._compile_failed = exc raise finally: self._expire_memoizations() diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 54b41fcc6..bab98f4fa 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -22,6 +22,7 @@ from sqlalchemy.orm.util import ( from sqlalchemy.orm.mapper import Mapper, _none_set from sqlalchemy.orm.unitofwork import UOWTransaction from sqlalchemy.orm import identity +import sys __all__ = ['Session', 'SessionTransaction', 'SessionExtension'] @@ -206,7 +207,9 @@ class SessionTransaction(object): single: thread safety; SessionTransaction """ - + + _rollback_exception = None + def __init__(self, session, parent=None, nested=False): self.session = session self._connections = {} @@ -229,9 +232,21 @@ class SessionTransaction(object): def _assert_is_active(self): self._assert_is_open() if not self._active: - raise sa_exc.InvalidRequestError( - "The transaction is inactive due to a rollback in a " - "subtransaction. Issue rollback() to cancel the transaction.") + if self._rollback_exception: + raise sa_exc.InvalidRequestError( + "This Session's transaction has been rolled back " + "due to a previous exception during flush." + " To begin a new transaction with this Session, " + "first issue Session.rollback()." + " Original exception was: %s" + % self._rollback_exception + ) + else: + raise sa_exc.InvalidRequestError( + "This Session's transaction has been rolled back " + "by a nested rollback() call. To begin a new " + "transaction, issue Session.rollback() first." + ) def _assert_is_open(self, error_msg="The transaction is closed"): if self.session is None: @@ -288,14 +303,16 @@ class SessionTransaction(object): assert not self.session._deleted for s in self.session.identity_map.all_states(): - _expire_state(s, s.dict, None, instance_dict=self.session.identity_map) + _expire_state(s, s.dict, None, + instance_dict=self.session.identity_map) def _remove_snapshot(self): assert self._is_transaction_boundary if not self.nested and self.session.expire_on_commit: for s in self.session.identity_map.all_states(): - _expire_state(s, s.dict, None, instance_dict=self.session.identity_map) + _expire_state(s, s.dict, None, + instance_dict=self.session.identity_map) def _connection_for_bind(self, bind): self._assert_is_active() @@ -379,7 +396,7 @@ class SessionTransaction(object): self.close() return self._parent - def rollback(self): + def rollback(self, _capture_exception=False): self._assert_is_open() stx = self.session.transaction @@ -397,6 +414,8 @@ class SessionTransaction(object): transaction._deactivate() self.close() + if self._parent and _capture_exception: + self._parent._rollback_exception = sys.exc_info()[1] return self._parent def _rollback_impl(self): @@ -415,7 +434,8 @@ class SessionTransaction(object): def close(self): self.session.transaction = self._parent if self._parent is None: - for connection, transaction, autoclose in set(self._connections.values()): + for connection, transaction, autoclose in \ + set(self._connections.values()): if autoclose: connection.close() else: @@ -1451,7 +1471,7 @@ class Session(object): ext.after_flush(self, flush_context) transaction.commit() except: - transaction.rollback() + transaction.rollback(_capture_exception=True) raise flush_context.finalize_flush_changes() diff --git a/test/ext/test_declarative.py b/test/ext/test_declarative.py index 26e1563fe..65ec92ca2 100644 --- a/test/ext/test_declarative.py +++ b/test/ext/test_declarative.py @@ -369,11 +369,14 @@ class DeclarativeTest(DeclarativeTestBase): hasattr(User.addresses, 'property') - # the exeption is preserved - - assert_raises_message(sa.exc.InvalidRequestError, - r"suppressed within a hasattr\(\)", - compile_mappers) + # the exception is preserved. Remains the + # same through repeated calls. + for i in range(3): + assert_raises_message(sa.exc.InvalidRequestError, + "^One or more mappers failed to initialize - " + "can't proceed with initialization of other " + "mappers. Original exception was: When initializing.*", + compile_mappers) def test_custom_base(self): class MyBase(object): diff --git a/test/orm/inheritance/test_single.py b/test/orm/inheritance/test_single.py index 420430954..2efde2b32 100644 --- a/test/orm/inheritance/test_single.py +++ b/test/orm/inheritance/test_single.py @@ -252,10 +252,12 @@ class RelationshipFromSingleTest(testing.AssertsCompiledSQL, MappedTest): 'employee_stuff_name, anon_1.employee_id ' 'AS anon_1_employee_id FROM (SELECT ' 'employee.id AS employee_id FROM employee ' - 'WHERE employee.type IN (?)) AS anon_1 ' + 'WHERE employee.type IN (:type_1)) AS anon_1 ' 'JOIN employee_stuff ON anon_1.employee_id ' '= employee_stuff.employee_id ORDER BY ' - 'anon_1.employee_id') + 'anon_1.employee_id', + use_default_dialect=True + ) class RelationshipToSingleTest(MappedTest): @classmethod diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index a19af5f4f..f041c8896 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -88,14 +88,25 @@ class MapperTest(_fixtures.FixtureTest): @testing.resolve_artifact_names def test_exceptions_sticky(self): - """test preservation of mapper compile errors raised during hasattr().""" + """test preservation of mapper compile errors raised during hasattr(), + as well as for redundant mapper compile calls. Test that + repeated calls don't stack up error messages. + + """ mapper(Address, addresses, properties={ 'user':relationship(User) }) hasattr(Address.user, 'property') - assert_raises_message(sa.exc.InvalidRequestError, r"suppressed within a hasattr\(\)", compile_mappers) + for i in range(3): + assert_raises_message(sa.exc.InvalidRequestError, + "^One or more mappers failed to " + "initialize - can't proceed with " + "initialization of other mappers. " + "Original exception was: Class " + "'test.orm._fixtures.User' is not mapped$" + , compile_mappers) @testing.resolve_artifact_names def test_column_prefix(self): @@ -157,13 +168,15 @@ class MapperTest(_fixtures.FixtureTest): @testing.resolve_artifact_names def test_column_not_present(self): - assert_raises_message(sa.exc.ArgumentError, "not represented in the mapper's table", mapper, User, users, properties={ - 'foo':addresses.c.user_id - }) + assert_raises_message(sa.exc.ArgumentError, + "not represented in the mapper's table", + mapper, User, users, properties={'foo' + : addresses.c.user_id}) @testing.resolve_artifact_names def test_bad_constructor(self): """If the construction of a mapped class fails, the instance does not get placed in the session""" + class Foo(object): def __init__(self, one, two, _sa_session=None): pass diff --git a/test/orm/test_session.py b/test/orm/test_session.py index b7ae104a1..6ac42a6b3 100644 --- a/test/orm/test_session.py +++ b/test/orm/test_session.py @@ -713,12 +713,39 @@ class SessionTest(_fixtures.FixtureTest): sess.flush() sess.rollback() assert_raises_message(sa.exc.InvalidRequestError, - 'inactive due to a rollback in a ' - 'subtransaction', sess.begin, - subtransactions=True) + "This Session's transaction has been " + r"rolled back by a nested rollback\(\) " + "call. To begin a new transaction, " + r"issue Session.rollback\(\) first.", + sess.begin, subtransactions=True) sess.close() @testing.resolve_artifact_names + def test_preserve_flush_error(self): + mapper(User, users) + sess = Session() + + sess.add(User(id=5)) + assert_raises( + sa.exc.DBAPIError, + sess.commit + ) + + for i in range(5): + assert_raises_message(sa.exc.InvalidRequestError, + "^This Session's transaction has been " + r"rolled back due to a previous exception during flush. To " + "begin a new transaction with this " + "Session, first issue " + r"Session.rollback\(\). Original exception " + "was:", + sess.commit) + sess.rollback() + sess.add(User(id=5, name='some name')) + sess.commit() + + + @testing.resolve_artifact_names def test_no_autocommit_with_explicit_commit(self): mapper(User, users) session = create_session(autocommit=False) |
