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()
