0.7 event migration
Kent
Is mapper event load() invoked at exactly the same place as the
deprecated reconstruct_instance() ?
Michael Bayer
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.
>
Kent Bower
Michael Bayer
Kent
"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??)
Michael Bayer
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??)
Kent
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:
def configure_attribute(class_, attr, inst):
"""
Set up function to be invoked when relations are 'get'ed on
possibly
transient objects, so we can automatically query these related
objects
"""
if isinstance(inst.property, RelationshipProperty):
default_loader = inst.impl.callable_
def load_value(state, passive):
if state.session_id is not None:
# this is persistent or pending, so
# return default sqla functionality
return default_loader(state, passive)
if passive is attributes.PASSIVE_NO_FETCH:
return attributes.PASSIVE_NO_RESULT
# session_id is currently None
state.session_id = DBSession().hash_key
retval = default_loader(state, passive)
state.session_id = None
return retval
inst.impl.callable_ = load_value
..
..
event.listen(DBEntity, 'attribute_instrument', configure_attribute)
> >>> --
> >>> 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.
Kent
state.session_id = None
in a finally block, but you get the idea
Michael Bayer
On Dec 26, 2011, at 1:50 PM, Kent wrote:
> 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.
Kent
> On Dec 26, 2011, at 1:50 PM, Kent wrote:
>
> > 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.
>
is preferable over the one I've currently got (callable_ seems more
public than _state_session). 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?
Michael Bayer
On Dec 27, 2011, at 8:10 AM, Kent wrote:
>
> 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.
Kent
to None should be in a finally: block)... and I would have assumed it
wasn't safe in the first place, but way back when I implemented it you
suggested it would work).
> 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.
>
porting to work out...
> 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.
in which non public hack I implemented... Think anyone else is doing
this? (I suspect several would like it, so making it easier/better is
good for the project.)
Thanks again, I'll let you know.
Kent
>
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?
Thanks again!
Michael Bayer
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?
Kent
So we'd have to move the state_session API invocation up (patch attached). Then this works as a proof of concept for both detached and transient objects.
I'm not sure if it was important for the "return attributes.ATTR_EMPTY" to have precedence over "return attributes.PASSIVE_NO_RESULT" so I kept them in that order to not disturb anything (otherwise we could delay the session lookup until after the "return attributes.PASSIVE_NO_RESULT" test). Those cases shouldn't be common anyway, right?
We'd also need to make this very local to the loading of attributes, as you mentioned, because object_session() also invokes _state_session()... we can't have that.. it messes up too much. (For example, transient objects appear pending and detached objects appear persistent, depending on your method of inspection.)
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?
=============================
diff -U10 -r sqlalchemy-default/lib/sqlalchemy/orm/strategies.py sqlalchemy-default.kb/lib/sqlalchemy/orm/strategies.py
--- sqlalchemy-default/lib/sqlalchemy/orm/strategies.py 2011-12-15 11:42:50.000000000 -0500
+++ sqlalchemy-default.kb/lib/sqlalchemy/orm/strategies.py 2011-12-27 17:48:54.000000000 -0500
@@ -450,41 +450,42 @@
self._rev_bind_to_col, \
self._rev_equated_columns
criterion = sql_util.adapt_criterion_to_null(criterion, bind_to_col)
if adapt_source:
criterion = adapt_source(criterion)
return criterion
def _load_for_state(self, state, passive):
- if not state.key and \
- (not self.parent_property.load_on_pending or not state.session_id):
+ prop = self.parent_property
+ pending = not state.key
+ session = sessionlib._state_session(state)
+
+ if pending and \
+ (not prop.load_on_pending or not session):
return attributes.ATTR_EMPTY
instance_mapper = state.manager.mapper
- prop = self.parent_property
key = self.key
prop_mapper = self.mapper
- pending = not state.key
if (
(passive is attributes.PASSIVE_NO_FETCH or \
passive is attributes.PASSIVE_NO_FETCH_RELATED) and
not self.use_get
) or (
passive is attributes.PASSIVE_ONLY_PERSISTENT and
pending
):
return attributes.PASSIVE_NO_RESULT
- session = sessionlib._state_session(state)
if not session:
raise orm_exc.DetachedInstanceError(
"Parent instance %s is not bound to a Session; "
"lazy load operation of attribute '%s' cannot proceed" %
(mapperutil.state_str(state), key)
)
# if we have a simple primary key load, check the
# identity map without generating a Query at all
if self.use_get:
Michael Bayer
On Dec 28, 2011, at 9:18 AM, Kent wrote:
>
> 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.
Michael Hipp
> detach(), also nice.
This seems most descriptive of what is actually taking place. I poured over the
docs for some time looking for the detach() method.
Michael
Kent
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)
in sqla 6.4,
bind_values = sql_util.bind_values(lazy_clause) would return the value
of the foreign key from the transient object
in sqla 7.5,
it returns [None], presumably because the committed values are not
set?
Short term, do you know right off what changed or what I could do to
work around this?
Michael Bayer
On Jan 9, 2012, at 2:30 PM, Kent wrote:
>> 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.
Kent Bower
Michael Bayer
On Jan 9, 2012, at 2:36 PM, Kent Bower wrote:
>> 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 ! :)
Kent Bower
already has that change from that ticket patched in. It must be
something else, I'm still looking...
Kent Bower
funny story, here's where it was added: http://www.sqlalchemy.org/trac/ticket/1910 which is essentially your ticket ! :)
Kent
Kent
Old code:
==============================================
def visit_bindparam(bindparam):
if bindparam.key in bind_to_col:
bindparam.value = lambda:
mapper._get_state_attr_by_column(
state, dict_,
bind_to_col[bindparam.key])
==============================================
New code (note that 'value' is now 'callable'):
def visit_bindparam(bindparam):
if bindparam._identifying_key in bind_to_col:
bindparam.callable = \
lambda: mapper._get_state_attr_by_column(
state, dict_,
bind_to_col[bindparam._identifying_key])
==============================================
Now look at sql.util.py:
==============================================
def bind_values(clause):
v = []
def visit_bindparam(bind):
value = bind.value
# evaluate callables
if callable(value):
value = value()
v.append(value)
visitors.traverse(clause, {}, {'bindparam':visit_bindparam})
return v
==============================================
Aren't we missing this: ?
if bind.callable:
value = bind.callable()
I think this is why it isn't loading the way it used to.