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
