Michael Bayer wrote:
I have committed a preliminary version of this change, which makes
the
central point of access within objectstore the "session", i.e.:
s = objectstore.session()
I think the implementation is still more complicated than it needs
to be. Here are some thoughts and a patch to make it simpler.
It was confusing to me that objectstore.uow was a function. The
same goes for objectstore.session. It looks like an instance. The
problem here is conventions: though objectstore is a module it is
presented as an object in the documentation. Accordingly,
objectstore.session looks like a property that returns a session
object when accessed. However, it's a function that returns a
Session instance when called (that's confusing). To make this more
intuitive I think we should rename "session(...)" to "get_session
(...)" (leave uow like it is because that's legacy now). The "get_"
prefix implies that it's a function not a property (attribute) of
objectstore.
I noticed that you're doing a thread-local thing with sessions.
That's fine. Let's make Session the main access point for
UnitOfWork functionality. UnitOfWork becomes an implementation
detail that is not directly exposed to (most) SA users. In
addition, the Session manages UnitOfWork instances with respect to
the UnitOfWork begin/commit cycle. UnitOfWork should not know about
the Session (or any type of session scoping mechanism)--it is a
unit of work, not a session manager. The logic in UnitOfWork.begin/
commit (that creates a "child" UnitOfWork and places it in the
registry) moves into Session, which is much cleaner. Aside: it is
up to the user to use thread-safe programming practices when using
Session objects directly. This should be simple now that Session is
a stateful object that does not know (or care) about the thread-
local pattern internally.
objectstore.get_session now provides access to a Session manager
that (by default) uses a thread-local pattern. This is a nice
feature provided by SA, but should not be tightly coupled with the
more powerful Session/UnitOfWork features. Thread-local session is
a common ORM data access pattern. However, it is only one of many
possible patterns. Before Session became a full-blown object,
SQLAlchemy did not allow other more complicated patterns to be used
without totally replacing the UnitOfWork management logic provided
by default.
High level overview of sqlalchemy.objectstore:
UnitOfWork - central SQLAlchemy object manager
- does not know about Session objects (Session manages UnitOfWork,
not the reverse)
Session - proxy for UnitOfWork. manages UnitOfWork begin/commit cycle
- does not know about session_registry or get_session
- when passing objects between threads, it is recommended to
"import_instance" into a thread-local session (Session behavior is
undefined if this recommendation is not observed).
session_registry - ScopedRegistry providing scoped access to SA-
managed Session objects
- defaults to thread-local session pattern
- the only reason a user would ever access this is to change the
default Session scope from "thread" to "application"
- does not directly deal with UnitOfWork instances (Session takes
care of that)
_sessions - private (weak value ref) dictionary of all Session
instances created by anyone
- provides a mechanism for a SQLAlchemy-managed object to weakly
reference a session (no more, no less)
get_session(obj=None, scope=None) - gets the session associated
with the given object or the scoped session (if obj is given, scope
has no affect. if neither is given, gets a session in the default
scope)
- is this too complicated? I'd really like it to do just one thing
- does not directly deal with UnitOfWork instances (Session takes
care of that)
uow - deprecated (use get_session)
- objectstore.uow.scope is now objectstore.session_registry.scope
using_session(session, func) - execute func with session default
scoped session
A patch that implements most of this is attached.
~ Daniel
Index: lib/sqlalchemy/mapping/mapper.py
===================================================================
--- lib/sqlalchemy/mapping/mapper.py (revision 942)
+++ lib/sqlalchemy/mapping/mapper.py (working copy)
@@ -205,7 +205,7 @@
oldinit = self.class_.__init__
def init(self, *args, **kwargs):
nohist = kwargs.pop('_mapper_nohistory', False)
- session = kwargs.pop('_sa_session',
objectstore.session())
+ session = kwargs.pop('_sa_session',
objectstore.get_session())
if oldinit is not None:
try:
oldinit(self, *args, **kwargs)
@@ -246,7 +246,7 @@
# store new stuff in the identity map
for value in imap.values():
- objectstore.session().register_clean(value)
+ objectstore.get_session().register_clean(value)
if len(mappers):
return [result] + otherresults
@@ -263,7 +263,7 @@
def _get(self, key, ident=None):
try:
- return objectstore.session()._get(key)
+ return objectstore.get_session()._get(key)
except KeyError:
if ident is None:
ident = key[2]
@@ -680,8 +680,8 @@
# including modifying any of its related items lists, as
its already
# been exposed to being modified by the application.
identitykey = self._identity_key(row)
- if objectstore.session().has_key(identitykey):
- instance = objectstore.session()._get(identitykey)
+ if objectstore.get_session().has_key(identitykey):
+ instance = objectstore.get_session()._get(identitykey)
isnew = False
if populate_existing:
Index: lib/sqlalchemy/mapping/objectstore.py
===================================================================
--- lib/sqlalchemy/mapping/objectstore.py (revision 942)
+++ lib/sqlalchemy/mapping/objectstore.py (working copy)
@@ -27,30 +27,25 @@
LOG = False
class Session(object):
- """a scope-managed proxy to UnitOfWork instances. Operations
are delegated
- to UnitOfWork objects which are accessed via a
sqlalchemy.util.ScopedRegistry object.
- The registry is capable of maintaining object instances on a
thread-local,
- per-application, or custom user-defined basis."""
+ """a proxy to UnitOfWork."""
- def __init__(self, uow=None, registry=None, hash_key=None):
+ def __init__(self, uowfunc=None, hash_key=None):
"""Initialize the objectstore with a UnitOfWork registry.
If called
with no arguments, creates a single UnitOfWork for all
operations.
- registry - a sqlalchemy.util.ScopedRegistry to produce
UnitOfWork instances.
- This argument should not be used with the uow argument.
- uow - a UnitOfWork to use for all operations. this
argument should not be
- used with the registry argument.
- hash_key - the hash_key used to identify objects against
this session, which
+ uowfunc - (optional) a callable that returns a clean
UnitOfWork when called. It
+ must accept one optional "parent" argument
+ hash_key - (optional) the hash_key used to identify
objects against this session.
defaults to the id of the Session instance.
-
"""
- if registry is None:
- if uow is None:
- uow = UnitOfWork(self)
- self.registry = util.ScopedRegistry(lambda:uow,
'application')
+ if uowfunc is None:
+ uowfunc = UnitOfWork
+ self.uowfunc = uowfunc
+ self.uow = uowfunc()
+ if hash_key is None:
+ self.hash_key = id(self)
else:
- self.registry = registry
- self._hash_key = hash_key
+ self.hash_key = hash_key
def get_id_key(ident, class_, table):
"""returns an identity-map key for use in storing/
retrieving an item from the identity
@@ -87,35 +82,54 @@
return (class_, table.hash_key(), tuple([row[column] for
column in primary_key]))
get_row_key = staticmethod(get_row_key)
- def _set_uow(self, uow):
- self.registry.set(uow)
- uow = property(lambda s:s.registry(), _set_uow, doc="Returns a
scope-specific UnitOfWork object for this session.")
-
- hash_key = property(lambda s:s._hash_key or id(s))
-
- def bind_to(self, obj):
- """given an object, binds it to this session. changes on
the object will affect
- the currently scoped UnitOfWork maintained by this
session."""
- obj._sa_session_id = self.hash_key
-
def __getattr__(self, key):
"""proxy other methods to our underlying UnitOfWork"""
- return getattr(self.registry(), key)
+ return getattr(self.uow, key)
+ 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.uow = self.uowfunc(parent=self.uow)
+
+ def commit(self):
+ """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 self.uow.parent is not None:
+ self.uow.commit()
+ self.uow = self.uow.parent
+ else:
+ self.uow.commit()
+
def clear(self):
- self.registry.clear()
+ """removes all current UnitOfWork instances from this
session and
+ establishes a new one. It is probably a good idea to
discard all
+ current mapped object instances, as they are no longer in
the Identity Map."""
+ self.uow = self.uowfunc()
+ def rollback(self):
+ if self.uow.parent is None:
+ raise "UOW transaction is not begun"
+ #self.uow.rollback() ??
+ self.uow = self.uow.parent
+
def delete(*obj):
"""registers the given objects as to be deleted upon the
next commit"""
- u = registry()
+ u = self.uow
for o in obj:
u.register_deleted(o)
+ def bind_to(self, obj):
+ """given an object, binds it to this session. changes on
the object will affect
+ the currently scoped UnitOfWork maintained by this
session."""
+ obj._sa_session_id = self.hash_key
+
def import_instance(self, instance):
"""places the given instance in the current thread's unit
of work context,
either in the current IdentityMap or marked as "new".
Returns either the object
or the current corresponding version in the Identity Map.
-
+
this method should be used for any object instance that is
coming from a serialized
storage, from another thread (assuming the regular
threaded unit of work model), or any
case where the instance was loaded/created corresponding
to a different base unitofwork
@@ -125,7 +139,7 @@
key = getattr(instance, '_instance_key', None)
mapper = object_mapper(instance)
key = (key[0], mapper.table.hash_key(), key[2])
- u = self.registry()
+ u = self.uow
if key is not None:
if u.identity_map.has_key(key):
return u.identity_map[key]
@@ -137,6 +151,13 @@
u.register_new(instance)
return instance
+ def register_clean(self, obj):
+ self.bind_to(obj)
+ self.uow.register_clean(obj)
+
+ def register_new(self, obj):
+ self.bind_to(obj)
+ self.uow.register_new(obj)
def get_id_key(ident, class_, table):
return Session.get_id_key(ident, class_, table)
@@ -147,53 +168,53 @@
def begin():
"""begins a new UnitOfWork transaction. the next commit will
affect only
objects that are created, modified, or deleted following the
begin statement."""
- session().begin()
+ get_session().begin()
def commit(*obj):
"""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."""
- session().commit(*obj)
+ get_session().commit(*obj)
def clear():
"""removes all current UnitOfWorks and IdentityMaps for this
thread and
establishes a new one. It is probably a good idea to discard all
current mapped object instances, as they are no longer in the
Identity Map."""
- session().clear()
+ get_session().clear()
def delete(*obj):
"""registers the given objects as to be deleted upon the next
commit"""
- s = session()
+ s = get_session()
for o in obj:
s.register_deleted(o)
def has_key(key):
"""returns True if the current thread-local IdentityMap
contains the given instance key"""
- return session().has_key(key)
+ return get_session().has_key(key)
def has_instance(instance):
"""returns True if the current thread-local IdentityMap
contains the given instance"""
- return session().has_instance(instance)
+ return get_session().has_instance(instance)
def is_dirty(obj):
"""returns True if the given object is in the current
UnitOfWork's new or dirty list,
or if its a modified list attribute on an object."""
- return session().is_dirty(obj)
+ return get_session().is_dirty(obj)
def instance_key(instance):
"""returns the IdentityMap key for the given instance"""
- return session().instance_key(instance)
+ return get_session().instance_key(instance)
def import_instance(instance):
- return session().import_instance(instance)
+ return get_session().import_instance(instance)
class UOWListElement(attributes.ListElement):
def __init__(self, obj, key, data=None, deleteremoved=False,
**kwargs):
attributes.ListElement.__init__(self, obj, key, data=data,
**kwargs)
self.deleteremoved = deleteremoved
def list_value_changed(self, obj, key, item, listval, isdelete):
- sess = session(obj)
+ sess = get_session(obj)
if not isdelete and sess.deleted.contains(item):
raise "re-inserting a deleted value into a list"
sess.modified_lists.append(self)
@@ -211,21 +232,15 @@
def value_changed(self, obj, key, value):
if hasattr(obj, '_instance_key'):
- session(obj).register_dirty(obj)
+ get_session(obj).register_dirty(obj)
else:
- session(obj).register_new(obj)
+ get_session(obj).register_new(obj)
def create_list(self, obj, key, list_, **kwargs):
return UOWListElement(obj, key, list_, **kwargs)
class UnitOfWork(object):
- def __init__(self, session, parent=None, is_begun=False):
- self.session = session
- self.is_begun = is_begun
- if is_begun:
- self.begin_count = 1
- else:
- self.begin_count = 0
+ def __init__(self, parent=None):
if parent is not None:
self.identity_map = parent.identity_map
else:
@@ -300,12 +315,10 @@
if not hasattr(obj, '_instance_key'):
mapper = object_mapper(obj)
obj._instance_key = mapper.instance_key(obj)
- self.session.bind_to(obj)
self._put(obj._instance_key, obj)
self.attributes.commit(obj)
def register_new(self, obj):
- self.session.bind_to(obj)
self.new.append(obj)
def register_dirty(self, obj):
@@ -331,18 +344,8 @@
pass
# TODO: tie in register_new/register_dirty with table
transaction begins ?
- def begin(self):
- if self.is_begun:
- self.begin_count += 1
- return
- u = UnitOfWork(self.session, parent=self, is_begun=True)
- self.session.registry.set(u)
def commit(self, *objects):
- if self.is_begun:
- self.begin_count -= 1
- if self.begin_count > 0:
- return
commit_context = UOWTransaction(self)
if len(objects):
@@ -396,20 +399,15 @@
e.commit()
commit_context.post_exec()
-
- if self.parent:
- self.session.registry.set(self.parent)
def rollback_object(self, obj):
self.attributes.rollback(obj)
def rollback(self):
- if not self.is_begun:
- raise "UOW transaction is not begun"
# roll back attributes ? nah....
#for obj in self.deleted + self.dirty + self.new:
# self.attributes.rollback(obj)
- self.session.registry.set(self.parent)
+ pass
class UOWTransaction(object):
"""handles the details of organizing and executing transaction
tasks
@@ -977,41 +975,47 @@
global_attributes = UOWAttributeManager()
-thread_session = Session(registry=util.ScopedRegistry(lambda:
UnitOfWork(thread_session), "thread"), hash_key='thread')
-uow = thread_session.registry # Note: this is not a UnitOfWork, it
is a ScopedRegistry that manages UnitOfWork objects
+session_registry = util.ScopedRegistry(Session, "thread") #
Default session registry
-_sessions = weakref.WeakValueDictionary()
-_sessions[thread_session.hash_key] = thread_session
+_sessions = weakref.WeakValueDictionary() # all referenced
sessions (including user-created)
-def session(obj=None):
- # object-specific session ?
+def get_session(obj=None, scope=None):
+ """returns a Session object
+
+ obj - if provided, the session referenced by this object will
be returned.
+ If the an associated session cannot be found the scoped
session will be
+ returned.
+ scope - if provided, the session in this scope will be returned
+
+ WARNING: this function may return a session from a different
thread if obj
+ is not associated with a thread-local session (use
Session.import_instance
+ to ensure that this does not happen)
+ """
if obj is not None:
- # does it have a hash key ?
hashkey = getattr(obj, '_sa_session_id', None)
if hashkey is not None:
- # ok, return that
try:
return _sessions[hashkey]
except KeyError:
- # oh, its gone, nevermind
pass
+ return session_registry(scope=scope)
+
+def uow():
+ print "WARNING: objectstore.uow is deprecated. use
objectstore.get_session"
+ return get_session().uow
- try:
- # have a thread-locally defined session (via using_session) ?
- return _sessions[thread.get_ident()]
- except KeyError:
- # nope, return the regular session
- return thread_session
+def using_session(session, func):
+ """call func with the given session as the default-scoped session
-def using_session(sess, func):
- old = _sessions.get(thread.get_ident(), None)
+ Note: the default-scoped session will always be restored even
if func
+ raises an exception.
+ """
+ old = session_registry.get()
try:
- _sessions[sess.hash_key] = sess
- _sessions[thread.get_ident()] = sess
+ session_registry.set(session)
return func()
finally:
if old is not None:
- _session[thread.get_ident()] = old
+ session_registry.set(old)
else:
- del _session[thread.get_ident()]
-
+ session_registry.clear()
Index: lib/sqlalchemy/util.py
===================================================================
--- lib/sqlalchemy/util.py (revision 942)
+++ lib/sqlalchemy/util.py (working copy)
@@ -377,12 +377,17 @@
self.defaultscope = scope
def __call__(self, scope=None):
+ """Get the object for the given scope, create a new one if
none exists"""
key = self._get_key(scope)
try:
return self.registry[key]
except KeyError:
return self.registry.setdefault(key, self.createfunc())
+ def get(self, scope=None):
+ """Get the object for the given scope, return None if none
exists"""
+ return self.registry.get(self._get_key(scope))
+
def set(self, obj, scope=None):
self.registry[self._get_key(scope)] = obj
