Skip to content

Commit 44ed81f

Browse files
refactor(robot-server): Avoid features that will be removed in SQLAlchemy 2.0 (#16926)
1 parent 1576581 commit 44ed81f

File tree

6 files changed

+57
-52
lines changed

6 files changed

+57
-52
lines changed

robot-server/pytest.ini

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,8 @@ markers =
44
ot3_only: Test only functions using the OT3 hardware
55
addopts = --color=yes --strict-markers
66
asyncio_mode = auto
7+
8+
# Don't allow any new code that uses features removed in SQLAlchemy 2.0.
9+
# We should remove this when we upgrade to SQLAlchemy 2.0.
10+
filterwarnings =
11+
error::sqlalchemy.exc.RemovedIn20Warning

robot-server/robot_server/persistence/_migrations/_util.py

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
"""Shared helpers for migrations."""
22

3-
import sqlalchemy
4-
53
import shutil
4+
import typing
65
from pathlib import Path
76

7+
import sqlalchemy
8+
89
from ..database import sqlite_rowid
910

1011

@@ -54,3 +55,43 @@ def copytree_if_exists(src: Path, dst: Path) -> None:
5455
shutil.copytree(src=src, dst=dst)
5556
except FileNotFoundError:
5657
pass
58+
59+
60+
def add_column(
61+
engine: sqlalchemy.engine.Engine,
62+
table_name: str,
63+
column: typing.Any,
64+
) -> None:
65+
"""Add a column to an existing SQL table, with an `ALTER TABLE` statement.
66+
67+
Params:
68+
engine: A SQLAlchemy engine to connect to the database.
69+
table_name: The SQL name of the parent table.
70+
column: The SQLAlchemy column object.
71+
72+
Known limitations:
73+
74+
- This does not currently support indexes.
75+
- This does not currently support constraints.
76+
- The column will always be added as nullable. Adding non-nullable columns in
77+
SQLite requires an elaborate and sensitive dance that we do not wish to attempt.
78+
https://www.sqlite.org/lang_altertable.html#making_other_kinds_of_table_schema_changes
79+
80+
To avoid those limitations, instead of this function, consider this:
81+
82+
1. Start with an empty database, or drop or rename the current table.
83+
2. Use SQLAlchemy's `metadata.create_all()` to create an empty table with the new
84+
schema, including the new column.
85+
3. Copy rows from the old table to the new one, populating the new column
86+
however you please.
87+
"""
88+
column_type = column.type.compile(engine.dialect)
89+
with engine.begin() as transaction:
90+
# todo(mm, 2024-11-25): This text seems like something that SQLAlchemy could generate for us
91+
# (maybe: https://docs.sqlalchemy.org/en/20/core/metadata.html#sqlalchemy.schema.Column.compile),
92+
# and that might help us account for indexes and constraints.
93+
transaction.execute(
94+
sqlalchemy.text(
95+
f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
96+
)
97+
)

robot-server/robot_server/persistence/_migrations/v3_to_v4.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@
99
from pathlib import Path
1010
from contextlib import ExitStack
1111
import shutil
12-
from typing import Any
13-
14-
import sqlalchemy
1512

13+
from ._util import add_column
1614
from ..database import sql_engine_ctx
1715
from ..file_and_directory_names import DB_FILE
1816
from ..tables import schema_4
@@ -35,16 +33,6 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None:
3533
dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file))
3634
schema_4.metadata.create_all(dest_engine)
3735

38-
def add_column(
39-
engine: sqlalchemy.engine.Engine,
40-
table_name: str,
41-
column: Any,
42-
) -> None:
43-
column_type = column.type.compile(engine.dialect)
44-
engine.execute(
45-
f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
46-
)
47-
4836
add_column(
4937
dest_engine,
5038
schema_4.analysis_table.name,

robot-server/robot_server/persistence/_migrations/v4_to_v5.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@
99
from pathlib import Path
1010
from contextlib import ExitStack
1111
import shutil
12-
from typing import Any
13-
14-
import sqlalchemy
1512

13+
from ._util import add_column
1614
from ..database import sql_engine_ctx
1715
from ..file_and_directory_names import DB_FILE
1816
from ..tables import schema_5
@@ -35,16 +33,6 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None:
3533
dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file))
3634
schema_5.metadata.create_all(dest_engine)
3735

38-
def add_column(
39-
engine: sqlalchemy.engine.Engine,
40-
table_name: str,
41-
column: Any,
42-
) -> None:
43-
column_type = column.type.compile(engine.dialect)
44-
engine.execute(
45-
f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
46-
)
47-
4836
add_column(
4937
dest_engine,
5038
schema_5.protocol_table.name,

robot-server/robot_server/persistence/_migrations/v6_to_v7.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@
1111
from pathlib import Path
1212
from contextlib import ExitStack
1313
import shutil
14-
from typing import Any
1514

1615
import sqlalchemy
1716

17+
from ._util import add_column
1818
from ..database import sql_engine_ctx, sqlite_rowid
1919
from ..tables import schema_7
2020
from .._folder_migrator import Migration
@@ -44,16 +44,6 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None:
4444

4545
dest_transaction = exit_stack.enter_context(dest_engine.begin())
4646

47-
def add_column(
48-
engine: sqlalchemy.engine.Engine,
49-
table_name: str,
50-
column: Any,
51-
) -> None:
52-
column_type = column.type.compile(engine.dialect)
53-
engine.execute(
54-
f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
55-
)
56-
5747
add_column(
5848
dest_engine,
5949
schema_7.run_command_table.name,

robot-server/robot_server/persistence/_migrations/v7_to_v8.py

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010
from pathlib import Path
1111
from contextlib import ExitStack
1212
import shutil
13-
from typing import Any
1413

1514
import sqlalchemy
1615

16+
from ._util import add_column
1717
from ..database import sql_engine_ctx
1818
from ..tables import schema_8
1919
from .._folder_migrator import Migration
@@ -41,16 +41,6 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None:
4141

4242
dest_transaction = exit_stack.enter_context(dest_engine.begin())
4343

44-
def add_column(
45-
engine: sqlalchemy.engine.Engine,
46-
table_name: str,
47-
column: Any,
48-
) -> None:
49-
column_type = column.type.compile(engine.dialect)
50-
engine.execute(
51-
f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
52-
)
53-
5444
add_column(
5545
dest_engine,
5646
schema_8.run_command_table.name,
@@ -73,9 +63,12 @@ def add_column(
7363
def _add_missing_indexes(dest_transaction: sqlalchemy.engine.Connection) -> None:
7464
# todo(2024-11-20): Probably add the indexes missing from prior migrations here.
7565
# https://opentrons.atlassian.net/browse/EXEC-827
76-
dest_transaction.execute(
77-
"CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run);"
66+
index = next(
67+
index
68+
for index in schema_8.run_command_table.indexes
69+
if index.name == "ix_run_run_id_command_status_index_in_run"
7870
)
71+
index.create(dest_transaction)
7972

8073

8174
def _migrate_command_table_with_new_command_error_col_and_command_status(

0 commit comments

Comments
 (0)