Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1j3bk7-0001CM-Ay for psycopg@arkaria.postgresql.org; Mon, 17 Feb 2020 08:27:03 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1j3bk6-0005gG-2D for psycopg@arkaria.postgresql.org; Mon, 17 Feb 2020 08:27:02 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1j3bk5-0005eW-J1 for psycopg@lists.postgresql.org; Mon, 17 Feb 2020 08:27:01 +0000 Received: from mail-io1-f50.google.com ([209.85.166.50]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1j3bjx-0002Ug-Mk for psycopg@postgresql.org; Mon, 17 Feb 2020 08:27:00 +0000 Received: by mail-io1-f50.google.com with SMTP id z8so5960120ioh.0 for ; Mon, 17 Feb 2020 00:26:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=wSE6UTish6OSru8Xi6mADuP83gxTeGiIMMXMqvnPRkU=; b=t0V2gfDsR4+lxEkn5Gmb1Bdq9v/k3jwxeouUD05ukXhsv6Xw4N6r8zO0oM6fKuuOV/ 1lkllz7G4FPONXQFoKhn/fi+j7ZPpCa1lYMXkichWHkeoKpF6IDg6XcUWEQPTCe8uhWN GDj7HIYBj5ccWBhbIOu5b0RJUc+/k4gdr/gzvURhvYyrwTcHdJyYBkgnSaPzFt99qNP3 OKRUjWswtrU4Ip4k8n30y7wQnic8R/er4+cW2+zI6LOhnleIwVczO2RAF5aEaTrOOSW1 oedC3GeuPBb0zezNUoKoyH6r3bvNDRVUBUmfEdcIueK5lmoEnXkdafmwiNQYK088kgt0 qqYw== X-Gm-Message-State: APjAAAU1zoBnHVjB1pTT6nnGijddLp/OVCHoDMqFLhtQZDYDKn5A/JUl wwt0NoTtARBYY34SLLTzm/ud4uOXxS7smBada2O4Nw== X-Google-Smtp-Source: APXvYqyRF1oNqBIVmVZ2HXq0CbPGvkLFpQE+ls9sssG+WDj61MexNyv1SvtWv2dbkj9fX6FLoAfzJWwi4bTbemIRY8c= X-Received: by 2002:a02:9f06:: with SMTP id z6mr11279963jal.2.1581928012230; Mon, 17 Feb 2020 00:26:52 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Daniel Fortunov Date: Mon, 17 Feb 2020 08:26:40 +0000 Message-ID: Subject: Re: Nested transactions support for code composability To: psycopg@postgresql.org Content-Type: multipart/alternative; boundary="0000000000003ab4bf059ec15195" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000003ab4bf059ec15195 Content-Type: text/plain; charset="UTF-8" > 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 wrote: > > > On Sat, 15 Feb 2020, 19:38 Daniel Fortunov, > 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 > > --0000000000003ab4bf059ec15195 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> Would it be possible to introduce this behaviour in psycopg2 in a w= ay that is not awful

I think there are two separate piec= es here that maybe can be treated independently:

1= . Introduction of conn.transaction() for explicit transaction management, i= ncluding support for nested transactions.
This addresses the firs= t 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 connec= tion)
This is certainly what one would intuitively expect (includ= ing me) -- I wasn't aware that `with conn` semantics for transaction ma= nagement 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 exce= ption, 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 introdu= ce `conn.transaction()` usage to existing code, whilst leaving management o= f connection lifetime the same as your code=C2=A0does it today. (This is ef= fectively what we have done in our codebase, by introducing `with Transacti= on(conn)` to existing code, regardless of the fact that `with conn` also do= es some transaction management.)

The only downside= of doing 1 without 2 is that this leaves the API design less clean, becaus= e now you have two ways to do transaction management, but I think this is p= referable 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" pat= tern even more (for psycopg2 at least), and introduce support for reentranc= y such that you can nest `with conn` blocks for savepoint-based nested tran= sactions. 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 implementati= on -- currently nested `with conn` blocks will result in the deepest block = committing (or rolling back) the entire transaction, and subsequent stateme= nt 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&quo= t; currently.)

Aside: Are you coming to PyCon US i= n 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.varrazzo@gmail.com> wrote:

On Sat, = 15 Feb 2020, 19:38 Daniel Fortunov, <postgresql@da= nielfortunov.com> wrote:
Based on the motivations outlined in the below mess= age to this list (3 years ago), I have implemented a Transaction context ma= nager that provides what I think is the most intuitive way to deal with tra= nsactions. (Functionally similar to xact / Django atomic, but available dir= ectly 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 rel= atively to your code/design, but rather relatively to the DBAPI requirement= s.

In short, there had b= een 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

<= br>
In hindsight, I think it was the wrong de= cision:

- people expects= Python objects to be closed on exit. Closing the tx but not the connection= is a surprising behaviour=C2=A0
- providing differe= nt connection factories, for instance a 'with pool.getconn():... ' = which would return the connection to the pool on exit=C2=A0- a rather elegant design -=C2=A0 would requi= re an extra level of with, see the thoughts in=C2=A0https://github.com/psycopg/psycopg2/pull/17

So my thoughts are mostly: what is the best i= nterface psycopg can present to offer:

- transactions when requested (on autocommit requests too, see=C2=A0https://github.com/psycopg/psycopg2/issues/941)

=C2=A0 =C2=A0= with connect('dsn') as conn:=C2=A0 # or with pool.getconn() etc.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 with conn.transaction() a= s tx:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with= conn.cursor() as curs:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 stuff()
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # conn.transaction() might be cal= led again to create savepoint tons
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # tx might expose commit()/rollback(= ) for explicit management=C2=A0

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # dispose if the cursor=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 # commit on success, ro= llback on error
=C2=A0 =C2=A0 # 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 backwar= ds compatible change, does the design above seem optimal (complete and easy= to use)?

For a full dis= closure: 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 act= ually become a real thing, if there is interest for it.

-- Daniele=C2=A0
--0000000000003ab4bf059ec15195--