Michael Bayer wrote:
dmiller wrote:
s1 = objectstore.get_session()
s2 = s1.begin()
# do stuff
s1.commit() # ERROR: uncommitted nested sessions
s2.commit() # not a real commit
s1.commit() # commit all changes including those made on s2
# do more stuff (all changes are now registered with s1)
s2.commit() # ERROR: session expired - parent session has been committed
s2 = s1.begin()
# do stuff
s2.commit() # ahh, that's better
s1.commit() # yeah, now you're getting it
Does this require two "commit" calls for every "begin" ? thats a little
strange.
No, but it takes less begins since the outer session does not need to be "begun". Sorry
for the confusion. However, while making a patch I realized that I needed a way to "end"
the nested environment because we can't use new session (objects would be associated with the wrong
session after the nested session was discarded). Here's an updated example:
def foo(s):
# do stuff
s.commit() # treat it like a normal session
# do more stuff
s.commit()
def bar(s):
# do other stuff
s.commit()
# make some changes that will not be committed
s = objectstore.get_session()
# do stuff - will not be committed by foo() or bar()
s.begin()
foo(s)
bar(s)
s.end()
s.commit() # commit stuff from before begin
Notice what's happening here:
1. foo() and bar can commit as many times as necessary (without a corresponding
begin). This is the same as if the function had been written with no intention
of being executed in a nested begin/commit environment. With the current model,
foo() would have to be rewritten to work in a nested environment (it would need
an extra begin before each commit, which is currently legal but not required
when operating in a non-nested environment).
2. A begin() is only needed before a session is passed into a nested
environment. This causes commits in the nested environment to only write
changes that are made within the nested environment.
3. An end() is needed for every begin()
4. This model has the added advantage of not requiring the developer to balance
begin/commit calls within the nested call. A begin() starts a nested session. A
commit on that nested session commits changes since that session began, but it
does not end the nested session. Multiple commits are legal after begin just
like in non-nested environments.
also it doesnt really prevent the error from happening, does it
? they still can say:
def func(s):
s2 = s1.begin()
# do stuff
s2.commit() # save changes up to here
# do more stuff
s1.commit() # save the remaining changes
which still does the s1.commit() that is supposed to happen outside of
func() ? are we just looking for things that make it more visually
obvious where the begins/commits occur ?
That is incorrect code. func() should be written as if it was getting an normal
(not nested) session. It does not need know that it is being executed in a
nested environment:
def func(s):
# do stuff
s.commit()
# do more stuff
s.commit()
The caller decides if func will be called in a nested scope. It could be called
this way (nested):
s = objectstore.get_session()
# do stuff - will not be committed by func
s.begin()
func(s)
s.end()
s.commit() # commit stuff done before calling func
...or this way (not nested):
s = objectstore.get_session()
func(s)
how about "begin()/commit()" for the two-phase version and "commit_all()"
for the one-phase, so that theyre just totally separate ?
I have implemented something like this (patch attached). The problem with
commit/commit_all is that code being written in a nested environment is
different than that being written in the outer environment (nested environments
would never call commit_all, correct?). The parent environment should be the
one calling begin/end while the child environment just uses the session like a
normal, non-nested session. That way any block of code that uses a session can
be nested by any other block of code that has a session.
I also changed Session.rollback slightly. It no longer depends on the begin/end
cycle (same as commit), and it now has the ability to rollback instances (also
like commit).
My only other suggestion is to change begin/end to begin_nested/end_nested, but
that seems too verbose so I didn't bother in my patch.
~ Daniel
Index: lib/sqlalchemy/mapping/objectstore.py
===================================================================
--- lib/sqlalchemy/mapping/objectstore.py (revision 972)
+++ lib/sqlalchemy/mapping/objectstore.py (working copy)
@@ -25,6 +25,9 @@
# printed to standard output. also can be affected by creating an engine
# with the "echo_uow=True" keyword argument.
LOG = False
+
+class SessionError(Exception):
+ pass
class Session(object):
"""Maintains a UnitOfWork instance, including transaction state."""
@@ -39,8 +42,7 @@
defaults to the id of the Session instance.
"""
self.uow = UnitOfWork()
- self.parent_uow = None
- self.begin_count = 0
+ self.uow_stack = []
self.nest_transactions = nest_transactions
if hash_key is None:
self.hash_key = id(self)
@@ -86,41 +88,38 @@
def begin(self):
"""begins a new UnitOfWork transaction. the next commit will affect
only
objects that are created, modified, or deleted following the begin
statement."""
- self.begin_count += 1
- if self.parent_uow is not None:
- return
- self.parent_uow = self.uow
+ self.uow_stack.append(self.uow)
self.uow = UnitOfWork(identity_map = self.uow.identity_map)
-
+
+ def end(self):
+ """ends a UnitOfWork transaction started by begin. all pending
changes on the
+ current UnitOfWork are discarded and the UnitOfWork that was in place
before
+ begin() was called is resumed. raises an exception if there was no
corresponding
+ begin"""
+ try:
+ self.uow = self.uow_stack.pop()
+ except IndexError:
+ raise SessionError("Session.end before Session.begin")
+
def commit(self, *objects):
"""commits the current UnitOfWork transaction. if a transaction was
begun
via begin(), commits only those objects that were created, modified,
or deleted
since that begin statement. otherwise commits all objects that have
been
changed.
- if individual objects are submitted, then only those objects are
committed, and the
- begin/commit cycle is not affected."""
- # if an object list is given, commit just those but dont
- # change begin/commit status
- if len(objects):
- self.uow.commit(*objects)
- return
- if self.parent_uow is not None:
- self.begin_count -= 1
- if self.begin_count > 0:
- return
- self.uow.commit()
- if self.parent_uow is not None:
- self.uow = self.parent_uow
- self.parent_uow = None
+ if individual objects are submitted, then only those objects are
committed"""
+ self.uow.commit(*objects)
- def rollback(self):
- """rolls back the current UnitOfWork transaction, in the case that
begin()
- has been called. The changes logged since the begin() call are
discarded."""
- if self.parent_uow is None:
- raise "UOW transaction is not begun"
- self.uow = self.parent_uow
- self.parent_uow = None
- self.begin_count = 0
+ def rollback(self, *objects):
+ """rolls back the current UnitOfWork transaction. all pending changes
logged since
+ the session started or since begin() was called (whichever was most
recent) are
+ discarded. objects with changes should either be discarded or placed
back in a
+ session with import_instance
+ if individual objects are submitted, then only attributes on those
objects are
+ rolled back"""
+ if objects:
+ self.uow.rollback(*objects)
+ else:
+ self.uow = UnitOfWork(identity_map = self.uow.identity_map)
def register_clean(self, obj):
self._bind_to(obj)
@@ -411,9 +410,10 @@
commit_context.post_exec()
- def rollback_object(self, obj):
- """'rolls back' the attributes that have been changed on an object
instance."""
- self.attributes.rollback(obj)
+ def rollback_object(self, *objects):
+ """'rolls back' the attributes that have been changed on the given
object instances."""
+ for obj in objects:
+ self.attributes.rollback(obj)
class UOWTransaction(object):
"""handles the details of organizing and executing transaction tasks
Index: test/objectstore.py
===================================================================
--- test/objectstore.py (revision 972)
+++ test/objectstore.py (working copy)
@@ -91,26 +91,48 @@
class User(object):pass
m = mapper(User, users)
def name_of(id):
- return users.select(users.c.user_id ==
id).execute().fetchone().user_name
- name1 = "Oliver Twist"
+ try:
+ return users.select(users.c.user_id ==
id).execute().fetchone().user_name
+ except:
+ return None
+ name1 = 'Oliver Twist'
name2 = 'Mr. Bumble'
+ name3 = 'The Thief'
self.assert_(name_of(7) != name1, msg="user_name should not be %s" %
name1)
self.assert_(name_of(8) != name2, msg="user_name should not be %s" %
name2)
+ self.assert_(name_of(9) != name3, msg="user_name should not be %s" %
name3)
s = objectstore.get_session()
- s.begin()
- s.begin()
m.get(7).user_name = name1
- s.begin()
+ s.begin() # begin nested
m.get(8).user_name = name2
+ s.begin() # begin second nested
+ m.get(9).user_name = name3
s.commit()
+ s.commit() # extra commit should have no affect
+ u = User() # create a user that will not be committed
+ u.user_id = 10
+ u.user_name = 'not persistent'
+ s.end()
self.assert_(name_of(7) != name1, msg="user_name should not be %s" %
name1)
self.assert_(name_of(8) != name2, msg="user_name should not be %s" %
name2)
+ self.assert_(name_of(9) == name3, msg="user_name should be %s" % name3)
+ self.assert_(name_of(10) == None, msg="user with user_id=10 should not
exist")
s.commit()
+ s.end()
self.assert_(name_of(7) != name1, msg="user_name should not be %s" %
name1)
- self.assert_(name_of(8) != name2, msg="user_name should not be %s" %
name2)
+ self.assert_(name_of(8) == name2, msg="user_name should be %s" % name2)
+ self.assert_(name_of(9) == name3, msg="user_name should be %s" % name3)
+ self.assert_(name_of(10) == None, msg="user with user_id=10 should not
exist")
s.commit()
self.assert_(name_of(7) == name1, msg="user_name should be %s" % name1)
self.assert_(name_of(8) == name2, msg="user_name should be %s" % name2)
+ self.assert_(name_of(9) == name3, msg="user_name should be %s" % name3)
+ self.assert_(name_of(10) == None, msg="user with user_id=10 should not
exist")
+ try:
+ s.end()
+ self.assert_(False, msg="s.end without s.begin should raise an
exception")
+ except objectstore.SessionError:
+ pass
class PKTest(AssertMixin):
