[ckan-dev] no autoflush in sqlalchemy

Mon Jan 17 15:37:29 UTC 2011

On Mon, Jan 17, 2011 at 3:07 PM, David Read <david.read at okfn.org> wrote:

> It seems to me a code simplification with a marginal performance cost
> - I'm in favour. Anyone else have a view?
>
> David
>

I forgot to mention that it increased the performance on the tests by about
5%.  There are less flushes altogether so is faster.

>
> On 17 January 2011 14:36, David Raznick <david.raznick at okfn.org> wrote:
> > Hello
> >
> > I propose we remove autoflush by default in ckan.  It caused a lot of
> bugs
> > on the version upgrade due to its magical behavior.
> >
> > I have a patch in https://bitbucket.org/kindly/ckan/    on branch
> > feature_switch_autoflush_off_by_default.
> >
> > This change simplifies code.  It only took 7 extra flushes/commits to
> make
> > work and there are over 40 instances that we remove autoflush, so as not
> to
> > cause error (I have not removed them in the patch yet).
> >
> > Things to know about using flush:
> >
> > * Most of the time nothing will change.   A commit implies flush.
> > * You only need to flush if you expect a query to return an object you
> just
> > saved.    i.e  if you  session.add(obj)  and you expect obj to be in a
> > session.query.
> > * A flush is handy when you want to get the primary key out before you do
> a
> > commit.  i.e   session.add(obj);  session.flush();  obj.id  is now
> > generated.
> > * You can use them as a limited form of nested transaction.
> >
> > I do not see any downsides.
> >
> > Thanks
> >
> > David
> >
> >
> >
> >
> >
> > _______________________________________________
> > ckan-dev mailing list
> > ckan-dev at lists.okfn.org
> > http://lists.okfn.org/mailman/listinfo/ckan-dev
> >
> >
>
> _______________________________________________
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
> http://lists.okfn.org/mailman/listinfo/ckan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.okfn.org/pipermail/ckan-dev/attachments/20110117/d431c402/attachment-0001.html>