[okfn-help] sqlalchemy model sessions

[Rufus Pollock]

John Bywater wrote:
> I've just committed some changes that make it all work again, following 
> this page:
> 
> http://www.sqlalchemy.org/docs/04/session.html
> 
> The main issue for me was that the created object needs committing to 
> the database before you get an id. You can do this by Session.commit() 
> but I'm not sure if you can rollback() afterwards. There are tests that 
> fail when the entity isn't committed, as it doesn't have an id. We might 
> need to manage things a bit more carefully than just a general 
> non-commit and general rollback (I think).

[snip]

After spending a significant amount of time working through this over on 
and off over the last few days I believe I have now have this pretty 
well worked out -- details below. Apart from the general insight into 
sqlalchemy's workings it also means all tests are now passing :)

~rufus

## Some General Points

1. We're using thread_local sessions

See:

   <http://www.sqlalchemy.org/docs/04/session.html#unitofwork_contextual>

We also associate classes and mappers with the contextual session and
use the auto-save feature. This means we do *not* need e.g. to do:

model.Session.save(obj) etc (usually)

2. transactional=True (when creating sessions)

NB: When using transactional=True you are *always* in a transaction. 
(more on this below in Transaction Confusion)

With transactional=True must call commit or nothing gets permanently
saved (unsurprisingly). Hence if you actually want stuff to be saved 
permanently to the db you *must* call commit() not just flush()

Note, however that calling flush is enough for things to be saved within
your session (i.e. you get ids, you can retrieve stuff etc)! But it will
only be inside a transaction and so won't result in changes to db unless
commit is called.

For transactions vs. flushes read:

   <http://www.sqlalchemy.org/docs/04/session.html#unitofwork_managing>

At the moment to reduce 'surpries' I've taken the approach of setting 
transactional=False which means that there are no transaction unless one 
is explicitly started. In this case, unless in txn, flush automatically 
writes all pending changes which is what we want.

NB: The alternative would be to explicitly make sure commit is called at
the end of every main 'cycle' (e.g. web request)


3. Think carefully about Session/session lifetimes

**This is where most of the testing confusion was coming from**

We now (correctly) do:

     model.Session.remove()

At the end of every __call__ on a controller (see lib/base.py)

Need to think carefully about the consequences of this when writing unit
tests for the following reason. sqlalchemy docs state [1]:

 > After remove() is called, the next call to Session() will create a new
 > Session object which then becomes the contextual session.

Now we currently had quite a bit of functional test code that did
something like:

     def setup_method(...): model.Session.begin()

         # create some stuff this material is implicitly associated with
         # the current model.Session()
         self.item = ...

         # or model.Session.commit()
         model.Session.flush()

     def some_test_method():
         # call to web app
         self.app.response(...)
         # inside web app model.Session.remove() is called this
         # automatically rollsback existing work

         # this will then be fine if we only have one call to web app
         # (Resulting in difficult to track down errors ...) but if a
         # second call inside this test then ...
         self.app.response(...)
         # EXCEPTION since everything now rolled back (so e.g. no id on
         # self.item etc)

         # NB: even if we do model.Session.commit() in setup things will
         # go wrong if we startedwith model.Session.begin since then this
         # rollback will still rollback the *outer* transaction within
         # which our explicitly started transaction is running See
         # Transaction Confusion for more on this

     def teardown_method(...):
         # has no effect ...
         model.Session.rollback()
         # this is essential as we were getting some pollution across
         # unrelated tests as we weren't fully removing Sessions at the
         # end of a given test
         model.Session.remove()

         # If we do wrapping everything in begin (which as just discussed
         # does not work here) need to delete stuff

         # try explicitly deleting things self.item.delete() will not
         # work as item is no longer associated with a Session this will
         # work model.Session.delete(self.item)

The long and the short of this is:

   * For anything testing the web app cannot not 'wrap' everything in a
     single transaction since the intermediate call to web app not only
     result in unmatched rollbacks but also 'remove' the Session.
   * Have to explicitly reattach fixtures to a Session in the teardown
     for deleting to work properly.

[1]:<http://www.sqlalchemy.org/docs/04/session.html#unitofwork_contextual>


## Transactional Confusion

if transactional=True for session then always in a transaction. Thus to
have anything save to db have to always have a commit somwhere at end of
everything:

Session.commit()

Without this do not need as things will be automatically saved at the
end of the session.

Up until now we had both transactional=True and were calling begin at
the start of each unittest. This led to confusion:

a) if just doing flush and no commits then even without explicit
rollback nothing would be saved.

b) if we were doing a commit weird stuff was happening since transaction
structure was

     T1 # started automatically by session

       T2 # started by explicit call to begin() in setup_class

       # ... commit somewhere in our code

       # automatically results in ending of current T and starting of a
       # new one

       T2 End T3 Start ...

       ... happens again and again TN

       rollback called in teardown TN rollback (perhaps having 0 effect)

     T1 still uncommitted so nothing has been saved to db!