On May 11, 2006, at 10:31 PM, Daniel Miller wrote:

I think by default it should require a session.save(u) before the flush (that's exactly what Hibernate does).

its the default.

Now if someone happens to want that type of magic, then they can use one of the _explicit_ recipes to get that behavior (i.e. the threadlocal mod (see below) or the base class or __session__ provided by SessionContext).

its been this way for awhile now.

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.

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 ?

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.


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 ? 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).

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)

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.

-                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.

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.

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.




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

Reply via email to