> On Nov 12, 2014, at 12:45 PM, Dan Smith <[email protected]> wrote: > >> I personally favour having consistent behaviour across the board. How >> about updating them all to auto-refresh by default for consistency, >> but adding an additional option to save() to disable it for particular >> calls? > > I think these should be two patches: one to make them all auto-refresh, > and another to make it conditional. That serves the purpose of (a) > bisecting a regression to one or the other, and (b) we can bikeshed on > the interface and appropriateness of the don't-refresh flag :) > >> I also suggest a tactical fix to any object which fetches itself twice >> on update (e.g. Aggregate). > > I don't see that being anything other than an obvious win, unless there > is some obscure reason for it. But yeah, seems like a good thing to do.
lets keep in mind my everyone-likes-it-so-far proposal for reader() and writer(): https://review.openstack.org/#/c/125181/ (this is where it’s going to go as nobody has -1’ed it, so in absence of any “no way!” votes I have to assume this is what we’re going with). in this system, the span of session use is implicit within the context and/or decorator, and when writer() is specified, a commit() can be implicit as well. IMHO there should be no “.save()” at all, at least as far as database writing is concerned. SQLAlchemy doesn’t need boilerplate like that - just let the ORM work normally: @sql.writer def some_other_api_method(context): someobject = context.session.query(SomeObject)….one() someobject.change_some_state(<stuff>) # done! if you want an explicit refresh, then just do so: @sql.writer def some_other_api_method(context): someobject = context.session.query(SomeObject)….one() someobject.change_some_state(<stuff>) context.session.flush() context.session.refresh(someobject) # do something with someobject however, seeing as this is all one API method the only reason you’d want to refresh() is that you think something has happened between that flush() and the refresh() that would actually show up, I can’t imagine what that would be looking for, unless maybe some large amount of operations took up a lot of time between the flush() and the refresh(). _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
