Re: [sqlalchemy] Re: 0.7 event migration

2012-01-10 Thread Kent Bower

funny story, here's where it was added:  
http://www.sqlalchemy.org/trac/ticket/1910 which is essentially your ticket !   
:)

I just double checked and I had patched in rfde41d0e9f70 
http://www.sqlalchemy.org/trac/changeset/fde41d0e9f70/.  Is there 
another commit that went against 1910?  For example, was there logic in 
the attachment /load_on_fks.patch/ 
http://www.sqlalchemy.org/trac/attachment/ticket/1910/load_on_fks.patch that 
was committed?


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



[sqlalchemy] Re: 0.7 event migration

2012-01-10 Thread Kent
See http://www.sqlalchemy.org/trac/ticket/2372

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



[sqlalchemy] Re: 0.7 event migration

2012-01-10 Thread Kent
Mike,

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.

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



Re: [sqlalchemy] Re: 0.7 event migration

2012-01-10 Thread Michael Bayer
Code wasn't covered and is a regresssion, fixed in rd6e321dc120d.


On Jan 10, 2012, at 10:58 AM, Kent wrote:

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

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



Re: [sqlalchemy] Re: 0.7 event migration

2012-01-10 Thread Kent Bower

Thank you very much!

On 1/10/2012 11:47 AM, Michael Bayer wrote:

Code wasn't covered and is a regresssion, fixed in rd6e321dc120d.


On Jan 10, 2012, at 10:58 AM, Kent wrote:


Mike,

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.

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



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



[sqlalchemy] Re: 0.7 event migration

2012-01-09 Thread Kent
 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)

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?

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



Re: [sqlalchemy] Re: 0.7 event migration

2012-01-09 Thread 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.


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



Re: [sqlalchemy] Re: 0.7 event migration

2012-01-09 Thread Kent Bower

On 1/9/2012 2:33 PM, Michael Bayer wrote:

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.


But this changed from 0.6.4?

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



Re: [sqlalchemy] Re: 0.7 event migration

2012-01-09 Thread 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 !   
:)


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



Re: [sqlalchemy] Re: 0.7 event migration

2012-01-09 Thread Kent Bower

On 1/9/2012 5:33 PM, Michael Bayer wrote:

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 !   
:)

Except that my patched version of 0.6.4 (which I was referring to) 
already has that change from that ticket patched in.  It must be 
something else, I'm still looking...


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



Re: [sqlalchemy] Re: 0.7 event migration

2011-12-28 Thread Kent

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.0 -0500
+++ sqlalchemy-default.kb/lib/sqlalchemy/orm/strategies.py  
2011-12-27 17:48:54.0 -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 

Re: [sqlalchemy] Re: 0.7 event migration

2011-12-28 Thread 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.



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



Re: [sqlalchemy] Re: 0.7 event migration

2011-12-28 Thread Michael Hipp

On 2011-12-28 10:58 AM, Michael Bayer wrote:

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

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



[sqlalchemy] Re: 0.7 event migration

2011-12-27 Thread Kent
On Dec 26, 5:12 pm, Michael Bayer mike...@zzzcomputing.com wrote:
 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.


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









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

Re: [sqlalchemy] Re: 0.7 event migration

2011-12-27 Thread 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.





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



[sqlalchemy] Re: 0.7 event migration

2011-12-27 Thread Kent
 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?

Thanks again!

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



Re: [sqlalchemy] Re: 0.7 event migration

2011-12-27 Thread Michael Bayer

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.


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



[sqlalchemy] Re: 0.7 event migration

2011-12-26 Thread Kent
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:

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)


On Dec 26, 12:26 pm, Michael Bayer mike...@zzzcomputing.com wrote:
 On Dec 26, 2011, at 9:07 AM, Kent wrote:

  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.

 yes that was a fabulous simplification of things I'm still very happy about.



  Can you briefly summarize how this is meant to work now? (I think the doc 
  string is wrong now??)

 docstring is wrong yes.   the callable just receives a state and a passive 
 flag, then loads something for the attribute.    It's just one less level of 
 callable and the two that we have in use are LoadDeferredColumns and 
 LoadLazyAttribute in strategies.py.     This also isn't very public API and 
 if I pointed you to this for some previous issue, I'd be curious if I 
 remembered to mention that.









  On 12/25/2011 10:31 AM, Michael Bayer wrote:

  yes a few change names, reconstruct_instance, init_instance, init_failed.

  On Dec 24, 2011, at 7:42 PM, Kent Bower wrote:

  Right.  And reconstruct_instance() was renamed load()?

  On 12/24/2011 5:56 PM, Michael Bayer wrote:
  On Dec 24, 2011, at 10:04 AM, Kent wrote:

  As the migration guide suggests, I'd like to embrace the events API.
  Is mapper event load() invoked at exactly the same place as the
  deprecated reconstruct_instance() ?
  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 sqlalchemy@googlegroups.com.
  To unsubscribe from this group, send email to 
  sqlalchemy+unsubscr...@googlegroups.com.
  For more options, visit this group 
  athttp://groups.google.com/group/sqlalchemy?hl=en.

  --
  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 
  athttp://groups.google.com/group/sqlalchemy?hl=en.

  --
  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 
  athttp://groups.google.com/group/sqlalchemy?hl=en.

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

[sqlalchemy] Re: 0.7 event migration

2011-12-26 Thread Kent
think I'll put:
state.session_id = None
in a finally block, but you get the idea

On Dec 26, 1:50 pm, Kent jkentbo...@gmail.com 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:

 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)

 On Dec 26, 12:26 pm, Michael Bayer mike...@zzzcomputing.com wrote:







  On Dec 26, 2011, at 9:07 AM, Kent wrote:

   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.

  yes that was a fabulous simplification of things I'm still very happy about.

   Can you briefly summarize how this is meant to work now? (I think the doc 
   string is wrong now??)

  docstring is wrong yes.   the callable just receives a state and a passive 
  flag, then loads something for the attribute.    It's just one less level 
  of callable and the two that we have in use are LoadDeferredColumns and 
  LoadLazyAttribute in strategies.py.     This also isn't very public API and 
  if I pointed you to this for some previous issue, I'd be curious if I 
  remembered to mention that.

   On 12/25/2011 10:31 AM, Michael Bayer wrote:

   yes a few change names, reconstruct_instance, init_instance, init_failed.

   On Dec 24, 2011, at 7:42 PM, Kent Bower wrote:

   Right.  And reconstruct_instance() was renamed load()?

   On 12/24/2011 5:56 PM, Michael Bayer wrote:
   On Dec 24, 2011, at 10:04 AM, Kent wrote:

   As the migration guide suggests, I'd like to embrace the events API.
   Is mapper event load() invoked at exactly the same place as the
   deprecated reconstruct_instance() ?
   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 sqlalchemy@googlegroups.com.
   To unsubscribe from this group, send email to 
   sqlalchemy+unsubscr...@googlegroups.com.
   For more options, visit this group 
   athttp://groups.google.com/group/sqlalchemy?hl=en.

   --
   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 
   athttp://groups.google.com/group/sqlalchemy?hl=en.

   --
   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 
   athttp://groups.google.com/group/sqlalchemy?hl=en.

-- 
You received this message because you are subscribed to the Google Groups 
sqlalchemy group.
To post to