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.
