On Dec 27, 10:39 am, Michael Bayer <mike...@zzzcomputing.com> wrote:
> 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.
>
Right, I wasn't happy about the state mutation (why the setting back
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.
>
Right, like I mentioned, if you ever change passive, I have more
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.
So, we're on the same page, I was just curious why you were interested
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.
--
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.
