Michael Bayer wrote:
On Apr 27, 2006, at 11:05 PM, Daniel Miller wrote:
the __session__() method is only called to get a session, *when the
instance is not already associated with a session*. this method is
merely there as the standard way to plug into systems such as the
SessionContext __metaclass__ youve built, which is associating
classes with a particular session context.
Actually, it's not doing that at all. It's associating instances with
a particular Session (not a SessionContext). OK, the base/meta classes
do (internally) associate the object with a context. But there should
be no need to obtain the context from an instance. When the current
session is needed it should be obtained directly from the context and
not through some arbitrary mapped-object instance. If a user needs to
pass a context around, then he should do that with his own plumbing.
It seems that you are saying you never want an instance added to a
Session automatically, in any case, ever - SA in all cases requires an
explicit save()/save-or-update(), and there is no way to change that
behavior, since there is no internal mechanism for a mapper to get a
session for an instance just created (recall that Mapper decorates a
classes' __init__ method to make this possible). correct ?
No, that's not what I want. But that made me see bug in the SessionContext
implementation, which may explain why you might think that. I have attached a
patch that fixes this bug...oh wait, I see that I've changed my mind about the
magic method you've proposed. I'll use that instead.
if that is the case, then we disagree; I believe there must be an
"implicit session" pattern available, never by default but very easy to
turn on without any extra programming, and should also be
customizable. This pattern seems to be very well accepted by the
current SA userbase and i dont think it should be removed:
mapper(User, user_table)
u = User() # 'u' is now associated with a session
I'm assume that implies a statement like install_mod("threadlocal") right? So
your example is really more like this:
install_mod("threadlocal")
class User(object):
...
m = mapper(User, user_table)
u = User() # 'u' is now associated with a session
Here's the SessionContext equivalent:
context = SessionContext()
class User(context.baseclass):
...
m = mapper(User, user_table)
u = User() # 'u' is now associated with session
The above could be using just the thread-local session, but i thought it
was a nice idea that you could also associate the User class with a
particular session context. otherwise i dont really understand the
point of having the baseclass and metaclass you proposed, it seems kind
of cruel to supply an extension that associates sessions with objects,
but then SA has no way to use it automatically.
You can associate the User class with whatever context instance you want by
using the baseclass or get_classmethod() (took the place of metaclass) of the
respective SessionContext instance.
[__session__() is] also not the kind of function people would
implement on a casual basis, its there to make a more advanced
concept possible in a clean, non-monkeypatching way.
perhaps naming __session__() as __sessioncontext__(), and having it
return a callable which then returns the contextual Session, will
make it clearer...though that seems more confusing.
From an OO perspective, allowing the contextual-session to be obtained
from an instance essentially gives that instance two sessions--the one
it is bound to and the context-current one. I think that's a
"Bozo-No-No" as my old CompSci prof used to say. Just because you can
doesn't mean you should... Really, I see no need for this. If the (SA)
framework really needs to know about the contextual session, then
provide a way to explicitly get a SessionContext instance rather than
using magic methods.
we can play rock-em-sock-em straw men if you want: I doubt your comp
sci class was teaching Python...what do you think he would say if you
told him "the effective type of an object should be determinable based
solely on the methods that happen to be available off of it" ? im sure
he would have flunked Guido.
I doubt it since C++ was the language used in the course...Guido is probably
more of an expert at that language than my prof was. But I digress :)
back to the issue at hand, the distinguishment to be made with the
__sessioncontext__ method, and i should make this clearer, is that we
are associating a session context with a class, not an instance. so
there is no "two sessions with an instance" going on, the instance has
one session, the class is optionally associated with a particular
context. the instance may contain a state that is specialized against
the default supplied by the class, and there is nothing wrong with
that. just like you associate a Mapper with the class, but an instance
might have been loaded by a different Mapper. it could be done the
opposite way, you can stick your SessionContext callable into the
mapper() call; same deal. Maybe that is a little clearer. although
then you cant easily associate a full class hierarchy with a particular
context...
OK, that makes a lot more sense. If it's a class method and not an instance
method, then it's very similar to the __init__ methods in SessionContext (I
think I like it even better). I implemented the SessionContext baseclass to use
that instead of a custom __init__.
If the method is returning a Session instance, then it should be called
__contextsession__. If it's called __sessioncontext__ then I would expect it to
return a SessionContext, not a Session. We should call it __contextsession__ so
we don't have to make the mapper (or any other part of SA that uses it) know
about the interface of SessionContext.
I've removed the metaclass in favor of a get_classmethod() method, which is
simpler and not quite as scary for those who aren't comfortable with
metaclasses. It's used like this:
class User(object):
__contextsession__ = context.get_classmethod()
The other option would be to formalize the interface of SessionContext (i.e.
move it into the orm.session module), and do it like this:
class User(object):
__sessioncontext__ = context
Then current_session() would just call obj.__sessioncontext__.current. Let me
know what you think.
~ Daniel
Index: lib/sqlalchemy/ext/sessioncontext.py
===================================================================
--- lib/sqlalchemy/ext/sessioncontext.py (revision 1366)
+++ lib/sqlalchemy/ext/sessioncontext.py (working copy)
@@ -1,118 +1,75 @@
-from sqlalchemy.util import ScopedRegistry
-
-class SessionContext(object):
- """A simple wrapper for ScopedRegistry that provides a "current" property
- which can be used to get, set, or remove the session in the current scope.
-
- By default this object provides thread-local scoping, which is the default
- scope provided by sqlalchemy.util.ScopedRegistry.
-
- Usage:
- engine = create_engine(...)
- def session_factory():
- return Session(bind_to=engine)
- context = SessionContext(session_factory)
-
- s = context.current # get thread-local session
- context.current = Session(bind_to=other_engine) # set current session
- del context.current # discard the thread-local session (a new one will
- # be created on the next call to context.current)
- """
- def __init__(self, session_factory, scopefunc=None):
- self.registry = ScopedRegistry(session_factory, scopefunc)
- super(SessionContext, self).__init__()
-
- def get_current(self):
- return self.registry()
- def set_current(self, session):
- self.registry.set(session)
- def del_current(self):
- self.registry.clear()
- current = property(get_current, set_current, del_current,
- """Property used to get/set/del the session in the current scope""")
-
- def create_metaclass(session_context):
- """return a metaclass to be used by objects that wish to be bound to a
- thread-local session upon instantiatoin.
-
- Note non-standard use of session_context rather than self as the name
- of the first arguement of this method.
-
- Usage:
- context = SessionContext(...)
- class MyClass(object):
- __metaclass__ = context.metaclass
- ...
- """
- try:
- return session_context._metaclass
- except AttributeError:
- class metaclass(type):
- def __init__(cls, name, bases, dct):
- old_init = getattr(cls, "__init__")
- def __init__(self, *args, **kwargs):
- session_context.current.save(self)
- old_init(self, *args, **kwargs)
- setattr(cls, "__init__", __init__)
- super(metaclass, cls).__init__(name, bases, dct)
- session_context._metaclass = metaclass
- return metaclass
- metaclass = property(create_metaclass)
-
- def create_baseclass(session_context):
- """return a baseclass to be used by objects that wish to be bound to a
- thread-local session upon instantiatoin.
-
- Note non-standard use of session_context rather than self as the name
- of the first arguement of this method.
-
- Usage:
- context = SessionContext(...)
- class MyClass(context.baseclass):
- ...
- """
- try:
- return session_context._baseclass
- except AttributeError:
- class baseclass(object):
- def __init__(self, *args, **kwargs):
- session_context.current.save(self)
- super(baseclass, self).__init__(*args, **kwargs)
- session_context._baseclass = baseclass
- return baseclass
- baseclass = property(create_baseclass)
-
-
-def test():
-
- def run_test(class_, context):
- obj = class_()
- assert context.current == get_session(obj)
-
- # keep a reference so the old session doesn't get gc'd
- old_session = context.current
-
- context.current = create_session()
- assert context.current != get_session(obj)
- assert old_session == get_session(obj)
-
- del context.current
- assert context.current != get_session(obj)
- assert old_session == get_session(obj)
-
- obj2 = class_()
- assert context.current == get_session(obj2)
-
- # test metaclass
- context = SessionContext(create_session)
- class MyClass(object): __metaclass__ = context.metaclass
- run_test(MyClass, context)
-
- # test baseclass
- context = SessionContext(create_session)
- class MyClass(context.baseclass): pass
- run_test(MyClass, context)
-
-if __name__ == "__main__":
- test()
- print "All tests passed!"
+from sqlalchemy.util import ScopedRegistry
+from sqlalchemy.orm.session import Session, object_session
+
+
+class SessionContext(object):
+ """A simple wrapper for ScopedRegistry that provides a "current" property
+ which can be used to get, set, or delete the session in the current scope.
+
+ By default this object provides thread-local scoping, which is the default
+ scope provided by sqlalchemy.util.ScopedRegistry.
+
+ Usage:
+ engine = create_engine(...)
+ def session_factory():
+ return Session(bind_to=engine)
+ context = SessionContext(session_factory)
+
+ s = context.current # get thread-local session
+ context.current = Session(bind_to=other_engine) # set current session
+ del context.current # discard the thread-local session (a new one will
+ # be created on the next call to context.current)
+ """
+ def __init__(self, session_factory=Session, scopefunc=None):
+ self.registry = ScopedRegistry(session_factory, scopefunc)
+ super(SessionContext, self).__init__()
+
+ def get_current(self):
+ return self.registry()
+ def set_current(self, session):
+ self.registry.set(session)
+ def del_current(self):
+ self.registry.clear()
+ current = property(get_current, set_current, del_current,
+ """Property used to get/set/del the session in the current scope""")
+
+ def get_classmethod(self):
+ """return a __contextsession__ classmethod to be assigned to a mapped
+ class.
+
+ Usage:
+ context = SessionContext(...)
+ class MyClass(object):
+ __contextsession__ = context.get_classmethod()
+ """
+ try:
+ return self._classmethod
+ except AttributeError:
+ def __contextsession__(cls):
+ return self.current
+ self._classmethod = classmethod(__contextsession__)
+ return self._classmethod
+
+ def create_baseclass(context):
+ """return a baseclass to be used by objects that wish to be bound to a
+ thread-local session upon instantiatoin.
+
+ The baseclass produced by this method is for convenience only. Its use
+ is not required.
+
+ Note non-standard use of 'context' rather than 'self' as the name of
+ the first arguement of this method.
+
+ Usage:
+ context = SessionContext(...)
+ class MyClass(context.baseclass):
+ ...
+ """
+ try:
+ return context._baseclass
+ except AttributeError:
+ class baseclass(object):
+ __contextsession__ = context.get_classmethod()
+ context._baseclass = baseclass
+ return baseclass
+ baseclass = property(create_baseclass)
Index: lib/sqlalchemy/orm/session.py
===================================================================
--- lib/sqlalchemy/orm/session.py (revision 1366)
+++ lib/sqlalchemy/orm/session.py (working copy)
@@ -424,8 +424,8 @@
_sessions = weakref.WeakValueDictionary()
def current_session(obj=None):
- if hasattr(obj.__class__, '__sessioncontext__'):
- return obj.__class__.__sessioncontext__()
+ if hasattr(obj.__class__, '__contextsession__'):
+ return obj.__class__.__contextsession__()
else:
return _default_session(obj=obj)
Index: test/sessioncontext.py
===================================================================
--- test/sessioncontext.py (revision 0)
+++ test/sessioncontext.py (revision 0)
@@ -0,0 +1,99 @@
+'''
+def test():
+ def run_test(class_, context):
+ obj = class_()
+ assert context.current == object_session(obj)
+
+ # keep a reference so the old session doesn't get gc'd
+ old_session = context.current
+
+ context.current = Session()
+ assert context.current != object_session(obj)
+ assert old_session == object_session(obj)
+
+ del context.current
+ assert context.current != object_session(obj)
+ assert old_session == object_session(obj)
+
+ obj2 = class_()
+ assert context.current == object_session(obj2)
+
+ # test metaclass
+ context = SessionContext(Session)
+ class MyClass(object): __contextsession__ = context.get_classmethod()
+ run_test(MyClass, context)
+
+ # test baseclass
+ context = SessionContext(Session)
+ class MyClass(context.baseclass): pass
+ run_test(MyClass, context)
+
+if __name__ == "__main__":
+ test()
+ print "All tests passed!"
+'''
+
+from testbase import PersistTest, AssertMixin
+import unittest, sys, os
+from sqlalchemy.ext.sessioncontext import SessionContext
+from sqlalchemy.orm.session import object_session
+from sqlalchemy import *
+import testbase
+
+db = testbase.db
+
+users = Table('users', db,
+ Column('user_id', Integer, Sequence('user_id_seq', optional=True),
primary_key = True),
+ Column('user_name', String(40)),
+ mysql_engine='innodb'
+)
+
+User = None
+
+class HistoryTest(AssertMixin):
+ def setUpAll(self):
+ db.echo = False
+ users.create()
+ db.echo = testbase.echo
+ def tearDownAll(self):
+ db.echo = False
+ users.drop()
+ db.echo = testbase.echo
+ def setUp(self):
+ clear_mappers()
+
+ def do_test(self, class_, context):
+ """test session assignment on object creation"""
+ obj = class_()
+ assert context.current == object_session(obj)
+
+ # keep a reference so the old session doesn't get gc'd
+ old_session = context.current
+
+ context.current = Session()
+ assert context.current != object_session(obj)
+ assert old_session == object_session(obj)
+
+ new_session = context.current
+ del context.current
+ assert context.current != new_session
+ assert old_session == object_session(obj)
+
+ obj2 = class_()
+ assert context.current == object_session(obj2)
+
+ def test_classmethod(self):
+ context = SessionContext()
+ class User(object): __contextsession__ = context.get_classmethod()
+ User.mapper = mapper(User, users)
+ self.do_test(User, context)
+
+ def test_baseclass(self):
+ context = SessionContext()
+ class User(context.baseclass): pass
+ User.mapper = mapper(User, users)
+ self.do_test(User, context)
+
+
+if __name__ == "__main__":
+ testbase.main()
