Michael Bayer wrote:
The mapper will have an optional session_context parameter. It will
provide the exact same functionality as current_session(), but
provides additional flexibility because each mapper can have it's own
context.
OK, so you want to put it in mapper, yes thats the other way to do it.
however. the "only one way to do it" guideline would suggest that, this
would be the only way to do it. i.e. we wouldnt have a __session__() or
__contextsession__() callable on classes anymore. because again people
will be confused, as you like to say.... do i make a class with
__session__()? or add a keyword argument to my mapper ?
i would say, the mapper keyword option (or similar, more on that below)
would be the only way this gets done, and rightly so since the mapper is
where you define your classes' interaction with SQLAlchemy's ORM. the
original philosophy of "your classes are mapped with no explicit
modifications whatsoever" is maintained.
Yes, that makes sense. Excellent!
having both a mapper argument for session context as well as a magic
class method that does the same thing would seem to fall directly in
your crosshairs of "ambiguous multiple methodology", does it ?
Yes. I was just realizing that as I sent the message last night. I decided not
to include it since it might make the proposal seem too radical for one email :)
So it sounds like we agree to get rid of __session__() (and even the
SessionContext baseclass and metaclass?) since they really are not needed if a
session context can be associated with a mapper, which makes more sense anyway.
another reason the mapper is probably the better place is because, not
sure if you noticed that I also implemented hibernate's "entity_name"
feature
(http://www.hibernate.org/hib_docs/v3/reference/en/html/mapping.html#mapping-entityname)
as far back as 0.1.6, meaning that a class can have multiple mapping
associations. it seems very natural that the association of a class to
a session context would happen in the same place, so that the same class
could be associated with multiple session contexts via multiple mappers.
That fits nicely. Looks like it was planned to work that way :)
With this approach it's not even necessary to reference the context
for every new mapper instantiation. The threadlocal mod will create a
single global SessionContext and then make this type of modification
to SA's mapper() function. That's even 100% backward compatible.
backwards compatible is always a good thing. so, we're monkeypatching
?
I don't think so, it's more of a wrapper than a monkeypatch... Can you
elaborate on what you mean here? Ahh, nevermind...I see you have a better
suggestion.
there is a potentially better way to do this, as there is already a
"global mapper extension" list in the mapper module that is used to
specify a list of MapperExtensions that are applied to all mappers (keep
in mind that MapperExtensions are chainable so this doesnt preclude
other extensions being used, or even overriding the default ones).
MapperExtension's interface could simply get a new method...lets
say...get_session_context(), and the mod would apply an appropriate
MapperExtension to the "default list". that would be how the default
session context is applied across the board, and its also overrideable
on a per-mapper basis (better than a straight monkeypatch).
Hmm, I hadn't thought of a mapper extension. Yes, that sounds good. If it was
implemented that way it could even be used independently from the threadlocal
mod.
my normal style would be to also have the session_context keyword
argument available on Mapper as shorthand for supplying a
MapperExtension, but if we think even thats too much variety, theres
other shorthand ways to specify a context to a mapper while keeping the
singular MapperExtension methdology (such as, mapper(class, table,
ext=sessioncontext.ext), where "ext" is a standard property on
SessionContext which returns a MapperExtension wired to return that
session context)
Are you referring to the "extension" parameter that mapper already has? Why not just put
the context mapper extension class in the same package as SessionContext? I'd vote for a slightly
more self-documenting property name like "mapper_extension"--then someone using
dir(context) would easily know what it was for. This would be a sort of equivalent to the metaclass
and baseclass properties of SessionContext, which won't be needed anymore with this new idea. Of
course it would still be possible to create a custom base class or metaclass for more obscure use
cases.
I have attached a patch. Note that I did not remove the
current_session() and other related functions from the orm.session
module.
calls to current_session() are now in exactly two places in the whole
codebase anyway. ive no allegance to it if a consistent replacement
methodology, such as the one above, is put into place.
Good, get rid of it. Let people create their own global registries (i.e.
SessionContext instances) or use the threadlocal mod where there is adequate
warning of what's happening behind the scenes.
- session = sessionlib.current_session(self)
+ session = self.session_context.current
now here is the interesting thing about saying
"session_context.current". If we say the interface is "a property
called 'current'", that suggests to me that SessionContext should be
part of SA core...because to me, when an interface suggests specific
named methods, and a class wtihin the core expects objects that obey
such an interface, there should at least be a no-op base class in the
core alongside it which serves as a guide to that contract, pretty much
analgous to a Java interface. also with regards to abstract interfaces,
im a little antsy about having a property be the abstract method rather
than just a regular method_name(), but I dont have a good explaination
for that...might be just my java-oriented bias.
I think if Java had real properties then the Hibernate developers would have
used a property in their CurrentSessionContext class. But the closest you can
get to a property in Java is to use normal methods: getXxx() and/or
setXxx(value).
the other way, just saying its a callable, which can be an object with a
__call__() method or just a function def, suggests to me that no
explicit class needs to be part of the SA core. since that interface is
based on a Python convention, rather than a specific named method. This
may be another reason why i like callables (the other reasons being, you
have the choice of using a def or a class, and also because WSGI is very
oriented around callables, which i think its slick). one kind of
interface is more formal and java-ish, the other is more minimal and
python-centric.
I agree, callable classes are cool, but they also look a bit misleading
sometimes--like the developer might have made a mistake:
context = SessionContext()
s = context() # whoa, that's an instance, not a function
Maybe that's just /my/ Java sentiments holding over...?
The only reason I can think of why we wouldn't want to use a property would be
to support versions of Python prior to 2.2 (before properties were introduced).
However, I think we'd have a lot of bigger problems if we tried to do that, so
that reason doesn't really hold up.
I also see what you mean about a callable being a more "natural" interface in
that it is the simplest possible interface. However, I also think there is value in
having a formal interface definition in the core. Even if it is just a callable, it
should be formally specified in the core if the core uses it at all.
however, i dont care. if we go with a named interface, which youve
supported all along, i might want to see the method as get_current()
(more traditional than a property) and id want an at least one-liner
SessionContext interface class (but more likely the whole thing since
its pretty short) in the session.py module that specifies it.
You'll have to do a better job of arguing against properties to convince me
that using them in an interface is such a bad thing. I'm really sorry, but I
just can't understand what's wrong with them--at their simplest they're just
normal attributes, which is even simpler than a method/function/callable. My
last argument for a property is to say it looks a bit more intuitive to me than
a plain callable:
s = session_context.current # there's no doubt what this does
s = session_context() # what are we getting here? maybe a new "session_context"
instance?
If it's a callable, then it should be a verb (i.e. get_session) to correctly imply what it does and
how it should be used. But it's a context object which is a noun. The "current" property
makes more sense because it implies that it references "the current session in the
context."
~ Daniel
-------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Sqlalchemy-users mailing list
Sqlalchemy-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sqlalchemy-users
