From 1f30b5a4e2e36f1a0e0499ac314ee8c2872075be Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 17 Apr 2023 16:31:29 +0100 Subject: [PATCH] db: Don't rely on branched connections We were previously calling 'connect()' on the 'connectable' object in 'run_migrations_online', regardless of whether it was an 'Engine' or 'Connection' object. This worked because, as noted in an inline comment, "when connectable is already a Connection object, calling 'connect()' gives us a *branched connection*." This is no longer the case. From the SQLAlchemy docs [1]: The Connection object does not support "branching", which was a pattern by which a sub "connection" would be used that refers to this connection as a parent. Update our code to reflect this change, using the newly updated example from the SQLAlchemy cookbook doc [2] as inspiration. [1] https://docs.sqlalchemy.org/en/14/core/future.html#sqlalchemy.future.Connection [2] https://alembic.sqlalchemy.org/en/latest/cookbook.html#connection-sharing Change-Id: I79f98930017f26492055c1295fc7900a2257ac68 Signed-off-by: Stephen Finucane --- nova/db/api/migrations/env.py | 16 +++++++++++----- nova/db/main/migrations/env.py | 16 +++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/nova/db/api/migrations/env.py b/nova/db/api/migrations/env.py index a74e135b2629..770d958647e6 100644 --- a/nova/db/api/migrations/env.py +++ b/nova/db/api/migrations/env.py @@ -95,13 +95,19 @@ def run_migrations_online(): prefix="sqlalchemy.", poolclass=pool.NullPool, ) + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + render_as_batch=True, + include_name=include_name, + ) - # when connectable is already a Connection object, calling connect() gives - # us a *branched connection*. - - with connectable.connect() as connection: + with context.begin_transaction(): + context.run_migrations() + else: context.configure( - connection=connection, + connection=connectable, target_metadata=target_metadata, render_as_batch=True, include_name=include_name, diff --git a/nova/db/main/migrations/env.py b/nova/db/main/migrations/env.py index 1f593aa561d2..3a872713c2ad 100644 --- a/nova/db/main/migrations/env.py +++ b/nova/db/main/migrations/env.py @@ -101,13 +101,19 @@ def run_migrations_online(): prefix="sqlalchemy.", poolclass=pool.NullPool, ) + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + render_as_batch=True, + include_name=include_name, + ) - # when connectable is already a Connection object, calling connect() gives - # us a *branched connection*. - - with connectable.connect() as connection: + with context.begin_transaction(): + context.run_migrations() + else: context.configure( - connection=connection, + connection=connectable, target_metadata=target_metadata, render_as_batch=True, include_name=include_name,