Re: Nested transactions support for code composability

> Would it be possible to introduce this behaviour in psycopg2 in a way
that is not awful

I think there are two separate pieces here that maybe can be treated
independently:

1. Introduction of conn.transaction() for explicit transaction management,
including support for nested transactions.
This addresses the first two out of the three desired interface features
you've listed, and is a non-breaking change.

2. The desire to replace `with conn` semantics with something more
intuitive (i.e. closing the connection)
This is certainly what one would intuitively expect (including me) -- I
wasn't aware that `with conn` semantics for transaction management were
adopted as a defacto DBAPI behaviour. I think this is a poor design choice
as it is not very intuitive.
Making __exit__ close the connection, however, is a breaking change and
would likely have to wait until psycopg3. The upgrade path for existing
code would also be horribly painful...
Perhaps a hybrid approach would be best: Have __exit__ commit an ongoing
transaction (if there is one) provided there was no exception, and then
close the connection. This would at least give you a clear error on the
next attempt to use the connection, and prevent data loss from closing a
connection with an uncommitted transaction.

In terms of dependencies, I think implementing 2 requires 1 (you need to
have a better transaction management construct to migrate existing `with
conn` code to), but 1 does not necessarily require 2. i.e. You can
introduce `conn.transaction()` usage to existing code, whilst leaving
management of connection lifetime the same as your code does it today.
(This is effectively what we have done in our codebase, by introducing
`with Transaction(conn)` to existing code, regardless of the fact that
`with conn` also does some transaction management.)

The only downside of doing 1 without 2 is that this leaves the API design
less clean, because now you have two ways to do transaction management, but
I think this is preferable to coupling 1 and 2 and only being able to
introduce both of them in psycopg3.

The only other avenue I can think of is to buy-in to the "`with conn` for
transaction management" pattern even more (for psycopg2 at least), and
introduce support for reentrancy such that you can nest `with conn` blocks
for savepoint-based nested transactions. This is the only approach I can
think of that maintains backward compatibility with psycopg2, but it has
all the same drawbacks of not being intuitive. (Sadly, actually, this is
only "backward compatible" in terms of "design spirit" not actually in
terms of implementation -- currently nested `with conn` blocks will result
in the deepest block committing (or rolling back) the entire transaction,
and subsequent statement execution in the outer block(s) beginning a new
transaction. We found a lot of weird edge cases like this when migrating
our codebase to use `with Transaction()` and having autocommit enabled by
default -- such cases that are probably conceptually "broken" but
nonetheless "work" currently.)

Aside: Are you coming to PyCon US in April? It would be great to discuss
some of these topics face-to-face.

Regards,
Daniel

On Sun, 16 Feb 2020 at 11:52, Daniele Varrazzo <daniele(dot)varrazzo(at)gmail(dot)com>
wrote:

>
>
> On Sat, 15 Feb 2020, 19:38 Daniel Fortunov, <postgresql(at)danielfortunov(dot)com>
> wrote:
>
>> Based on the motivations outlined in the below message to this list (3
>> years ago), I have implemented a Transaction context manager that provides
>> what I think is the most intuitive way to deal with transactions.
>> (Functionally similar to xact / Django atomic, but available directly on
>> psycopg2 connections)
>>
>
> ...
>
>
>> Is this worthy of inclusion in psycopg2.extras (or elsewhere in psycopg2?)
>>
>
> Hello Daniel, thank you for the idea, and I would like to provide such
> functionality in psycopg.
>
> My doubts are around the interface: not so much relatively to your
> code/design, but rather relatively to the DBAPI requirements.
>
> In short, there had been an emerging pattern of drivers using the
> connection context manager to manage transactions (see e.g. psycopg
> behaviour: https://www.psycopg.org/docs/usage.html#with-statement) This
> is sort of a de-facto behaviour, but it never got written down in the DBAPI
> specs
>
> https://mail.python.org/pipermail/db-sig/2012-November/thread.html
>
> In hindsight, I think it was the wrong decision:
>
> - people expects Python objects to be closed on exit. Closing the tx but
> not the connection is a surprising behaviour
> - providing different connection factories, for instance a 'with
> pool.getconn():... ' which would return the connection to the pool on exit -
> a rather elegant design - would require an extra level of with, see the
> thoughts in https://github.com/psycopg/psycopg2/pull/17
>
> So my thoughts are mostly: what is the best interface psycopg can present
> to offer:
>
> - transactions when requested (on autocommit requests too, see
> https://github.com/psycopg/psycopg2/issues/941)
> - nested transactions
> - a non surprising behaviour on __exit__
>
> In my ideal world, the behaviour should be something like:
>
> with connect('dsn') as conn: # or with pool.getconn() etc.
> with conn.transaction() as tx:
> with conn.cursor() as curs:
> stuff()
> # conn.transaction() might be called again to create
> savepoint tons
> # tx might expose commit()/rollback() for explicit
> management
>
> # dispose if the cursor
> # commit on success, rollback on error
> # close the connection, return it to the pool, etc
>
> Would it be possible to introduce this behaviour in psycopg2 in a way that
> is not awful, which wouldn't break programs written for the 2.5-2.8
> behaviour? I don't see an obvious way to do it.
>
> If not, and we had to introduce a non backwards compatible change, does
> the design above seem optimal (complete and easy to use)?
>
> For a full disclosure: in the next months my work situation should change
> and I might be able to start working on a new major psycopg version, so
> psycopg3 might actually become a real thing, if there is interest for it.
>
> -- Daniele
>
>