yeah nothing has been moved. All the places where the internals would call XXXExtension.xxx_event() were just replaced with self.dispatch.xxx_event(), and the old Extension classes are invoked via an adapter to the new system. All unit tests for the extension system remain in place and haven't been modified.
>
> --
> You received this message because you are subscribed to the Google Groups "sqlalchemy" group.
> To post to this group, send email to sqlal...@googlegroups.com.
> To unsubscribe from this group, send email to sqlalchemy+...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.
>
Documentation for AttributeImpl.callable_ still reads
"optional function which generates a callable based on a parent
instance, which produces the "default" values for a scalar or
collection attribute when it's first accessed, if not present
already."
But it seems it is no longer the function which generates a callable, but rather is the callable itself now, accepting both the state and the passive parameters.
It used to be two stages, first callable_ accepts a state and then that returns a callable which accepted the passive parameter.
Can you briefly summarize how this is meant to work now? (I think the doc string is wrong now??)
> Yes, a nice simplification.
> I'm using it to lazyload attributes for objects that aren't in a
> session. I'm not sure if you pointed me there, I think I found it
> myself, but you helped work out the later details...
> Our app lives inside a webserver framework that, very appropriately,
> in my opinion, only has one session for any given web request. So,
> for our framework, I can safely lazyload related attributes for
> transient or detached objects by temporarily setting state.session_id:
heh yes this is exactly what things used to do between like version 0.2 and 0.4 :) - if there was no session, it would look in the "threadlocal" context which somehow was associated with things. This was one of many implicit decisions/surprises we took out in favor of a behavioral model that's transparent. For awhile there we also had this crazy "session lookup" logic that was quasi-optional, nobody knew what it was, and I had similar endless threads with a few interested folks like yourself trying to explain/rationalize it, as well as try to decide how this could be an "optional" hook. The library only improved as I took out all kinds of tricks like this which were leftovers from the original 0.1 model that followed the "magic everything, everywhere" pattern.
various things come to mind in terms of re-enabling this pattern.
One is that the "session registry" would be extensible. Technically it is right now if you were to override orm.session._state_session(). Then the lazyloaders would treat the parent object as though it were inside of whatever session you had it returning here. You could even hardwire it to the contextual session itself - then even sharing objects among threads would be selecting the current thread's session. I could barely consider what level of surprise side effects might/might not occur from this practice, but it is how things used to work.
If you want to play with it, I can re-add some kind of event for this. I have a name like "detached session" in my mind, maybe an event that's only called if there's no session_id, maybe it's just a session registry you stick onto a mapper, though I suppose the event hook is a good place here.
The other is that the "loader callable" here is a lot like an event and we could put a hook here that you'd just listen() for. That wouldn't be very difficult to do, though determining when it's invoked would be tricky. When the value is locally present in __dict__, I wouldn't want even one function call of latency there checking for a possible event handler. When the value isn't present, would the event be before callable_() is invoked, or only if callable_() is not present, etc. Also i played around with how the "load_lazy" function could be exposed such that the Session could be supplied, but it's not very nice for a public API - some loader callables populate the attribute, others return the function and the AttributeImpl does the population. Exposing this publicly would mean I can't adjust that without the risk of breaking existing apps.
So see what happens if you, for the moment, just monkeypatch over orm.session._state_session to do a lookup in a global context if state.session_id isn't set. If that solves the problem of "I want detached objects to load stuff", for you and everyone else who wants this feature, then whatever - I'm not at all thrilled about this use case but if it's just one trivial hook that I don't need to encourage, then there you go.
>
> Sounds good, I'll look into that. But I'm curious why this approach
> is preferable over the one I've currently got (callable_ seems more
> public than _state_session).
callable_ does not accept the "session" as an argument, and while I looked into ways of exposing a similar "callable_" that does accept a Session, it didn't produce a feature that would make much sense to people, nor of be much use; the only need to get at the callable_ is to enable the hack you have here, which is entirely so that lazy loaders work on detached objects.
Alternatively, there's already a well known behavior that we used to have up until version 0.4, which is to link the master session registry with an arbitrary contextual one, which we can quietly re-enable. Having you test this with the non-public _state_session is just a proof of concept.
> I admit, it would free my code from the
> passive logic, since I know you were considering changing those to bit
> flags, but why is this callable_ approach bad?
this is essentially the pattern:
def do_something(object):
my_important_thing = object.my_important_thing
if not my_important_thing:
raise Exception("Object doesn't have my important thing!")
myobject = get_some_object()
important_thing = get_some_important_thing()
myobject.my_important_thing = important_thing
do_something(object)
myobject.my_important_thing = None
We'd prefer to just pass two arguments:
def do_something(object, my_important_thing):
# ...
myobject = get_some_object()
important_thing = get_some_important_thing()
do_something(myobject, important_thing)
That way we aren't mutating state, which is never preferred, and we allow the signature of do_something() to ensure the correct state instead of resorting to manual extraction and argument checking. Your current approach is a "hack" because it makes it obvious that the public API was not designed for what you're doing and then resorts to state mutation in order to pass an argument.
At another level, it also exposes the mechanics of attribute lazy loading inside your application. It's simpler on your end for these details to not be necessary.
Within SQLAlchemy, our methods DeferredLoader._load_for_state and LazyLoader._load_for_state are fixed to a specific use case, where we assume "session" is on the object and it's an error otherwise, then as a second step we do a loading operation with this session. For the API change I tried, part of it involves breaking out the second half of each _load_for_state into a method like def _lazy_load(self, session, state, passive), so that the validation rule of "extract session and ensure it's present" and the operation of "load something" are separate. As SQLA itself doesn't need _lazy_load() separately right now it's not necessary and we have a more coarse grained method, but if we wanted to expose the callable_() in a more open ended way this would be part of the change.
So see what happens if you, for the moment, just monkeypatch over orm.session._state_session to do a lookup in a global context if state.session_id isn't set. If that solves the problem of "I want detached objects to load stuff", for you and everyone else who wants this feature, then whatever - I'm not at all thrilled about this use case but if it's just one trivial hook that I don't need to encourage, then there you go.
Please give me a lesson in monkeypatching 101. This isn't being
invoked:
=====================
import sqlalchemy.orm.session as session_module
sqla_state_session = session_module._state_session
def _state_session(state):
"""
for transient/detached objects, so we can automatically query
these related objects
"""
return sqla_state_session(state) or DBSession()
setattr(session_module, '_state_session', _state_session)
=====================
I presume there are already references to _state_session before I
change it.
Also, will this cause other side effects, such as obj in DBSession
reporting True when it used to report False or the orm internals being
confused by the return of this value?
>
> Off topic, but from a shell prompt I sometimes find myself naturally attempting this:
> session.detach(instance)
>
> and then when that fails, I remember:
> session.expunge(instance)
>
> I'm not asking for a change here, but quite curious: you think 'detach' is a better/more natural term?
I'll agree I hate the term "expunge()". "evict()" is often what I think of. detach(), also nice. consider that it's the opposite of add(). unfortunately remove() is already taken.
i guess the patch is interacting with that "load_on_pending" stuff, which I probably added for you also. It would be nice to really work up a "new SQLAlchemy feature: detached/transient object loading" document that really describes what it is we're trying to do here. If you were to write such a document, what example would you give as the rationale ? I know that's the hard part here, but this is often very valuable, to look at your internal system and genericize it into something universally desirable. It would make it clearer what we'd do with the flush() issue from yesterday too.
This seems most descriptive of what is actually taking place. I poured over the
docs for some time looking for the detach() method.
Michael
>> i guess the patch is interacting with that "load_on_pending" stuff, which I probably added for you also. It would be nice to really work up a "new SQLAlchemy feature: detached/transientobject loading" document that really describes what it is we're trying to do here. If you were to write such a document, what example would you give as the rationale ? I know that's the hard part here, but this is often very valuable, to look at your internal system and genericize it into something universally desirable.
>
> As far as such a document, would you want a trac ticket opened with my
> use case in a generalized form where others may likely have the same
> use case?
>
> Hoping to not upset you here.....:
>
> My "AttributeImpl.callable_" hack to set a transient state's
> session_id, load the relationship, and then set it back to None works
> for m2o but when it needs to load a collection (or it can't use get()
> I presume), then I am hitting this "return None":
>
> class LazyLoader(AbstractRelationshipLoader):
> def _load_for_state(self, state, passive):
> ...
> ...
> lazy_clause = strategy.lazy_clause(state)
>
> if pending:
> bind_values = sql_util.bind_values(lazy_clause)
> if None in bind_values:
> return None ### <================
>
> q = q.filter(lazy_clause)
that means some of the columns being linked to the foreign keys on the target are None. If you want your lazyload to work all the attributes need to be populated. If you're hitting the "get committed " thing, and the attributes are only pending, then that's what that is.
>> that means some of the columns being linked to the foreign keys on the target are None. If you want your lazyload to work all the attributes need to be populated. If you're hitting the "get committed " thing, and the attributes are only pending, then that's what that is.
>>
> But this changed from 0.6.4?
funny story, here's where it was added: http://www.sqlalchemy.org/trac/ticket/1910 which is essentially your ticket ! :)
funny story, here's where it was added: http://www.sqlalchemy.org/trac/ticket/1910 which is essentially your ticket ! :)