On 12/27/2011 5:34 PM, Michael Bayer wrote:

On Dec 27, 2011, at 5:21 PM, Kent wrote:

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.

Hm I would have guessed, but mapper.py and strategies.py seem to be calling it relative to the module. What does pdb tell you if you post mortem into where it raises the Detached error ?


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?

maybe ? if it doesn't just work then yes. The new logic here only needs to take place at the point at which it loads an attribute so if we made it local to those areas, there shouldn't be any big issues.

I'm basically making you a developer here to help me test this out.



ok, it was never making it that far because my main use case for this is to work with transient instances (detached are secondary). I was testing with a transient object and it never got past the first if statement in _load_for_state(), which was using session_id instead of the return value from _state_session().

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:

--
You received this message because you are subscribed to the Google Groups 
"sqlalchemy" group.
To post to this group, send email to sqlalchemy@googlegroups.com.
To unsubscribe from this group, send email to 
sqlalchemy+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/sqlalchemy?hl=en.

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:

Reply via email to