Skip to content

Commit

Permalink
Use NullPool for sqlite engines
Browse files Browse the repository at this point in the history
This restores the behavior under SQLAlchemy 1.4
(Note that we set the pool for sqlite only if it's not an in-memory db
  • Loading branch information
jdavcs committed Mar 5, 2024
1 parent 8c88bf9 commit b644b1d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 23 deletions.
18 changes: 11 additions & 7 deletions lib/galaxy/model/orm/engine_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
exc,
)
from sqlalchemy.engine import Engine
from sqlalchemy.pool import NullPool

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -101,8 +102,13 @@ def after_cursor_execute(conn, cursor, statement, parameters, context, executema
pass

engine_options = engine_options or {}
engine_options = set_sqlite_connect_args(engine_options, url)
engine = create_engine(url, **engine_options, future=True)
if url.startswith("sqlite://"):
set_sqlite_connect_args(engine_options, url)

if url.startswith("sqlite://") and url not in ("sqlite:///:memory:", "sqlite://"):
engine = create_engine(url, **engine_options, poolclass=NullPool, future=True)
else:
engine = create_engine(url, **engine_options, future=True)

# Prevent sharing connection across fork: https://docs.sqlalchemy.org/en/14/core/pooling.html#using-connection-pools-with-multiprocessing-or-os-fork
register_after_fork(engine, lambda e: e.dispose())
Expand All @@ -123,13 +129,11 @@ def checkout(dbapi_connection, connection_record, connection_proxy):
return engine


def set_sqlite_connect_args(engine_options: Dict, url: str):
def set_sqlite_connect_args(engine_options: Dict, url: str) -> None:
"""
Add or update `connect_args` in `engine_options` if db is sqlite.
Set check_same_thread to False for sqlite, handled by request-specific session.
See https://fastapi.tiangolo.com/tutorial/sql-databases/#note
"""
if url.startswith("sqlite://"):
connect_args = engine_options.setdefault("connect_args", {})
connect_args["check_same_thread"] = False
return engine_options
connect_args = engine_options.setdefault("connect_args", {})
connect_args["check_same_thread"] = False
32 changes: 16 additions & 16 deletions test/unit/data/model/test_engine_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,28 @@
class TestSetSqliteConnectArgs:
def test_engine_options_empty(self):
engine_options = {} # type: ignore[var-annotated]
updated = set_sqlite_connect_args(engine_options, SQLITE_URL)
assert updated == {"connect_args": {"check_same_thread": False}}
set_sqlite_connect_args(engine_options, SQLITE_URL)
assert engine_options == {"connect_args": {"check_same_thread": False}}

def test_update_nonempty_engine_options(self):
engine_options = {"foo": "some foo"}
updated = set_sqlite_connect_args(engine_options, SQLITE_URL)
assert len(updated) == 2
assert updated["foo"] == "some foo"
assert updated["connect_args"] == {"check_same_thread": False}
set_sqlite_connect_args(engine_options, SQLITE_URL)
assert len(engine_options) == 2
assert engine_options["foo"] == "some foo"
assert engine_options["connect_args"] == {"check_same_thread": False} # type:ignore[comparison-overlap]

def test_overwrite_connect_args(self):
engine_options = {"foo": "some foo", "connect_args": {"check_same_thread": True}}
updated = set_sqlite_connect_args(engine_options, SQLITE_URL)
assert len(updated) == 2
assert updated["foo"] == "some foo"
assert updated["connect_args"] == {"check_same_thread": False}
set_sqlite_connect_args(engine_options, SQLITE_URL)
assert len(engine_options) == 2
assert engine_options["foo"] == "some foo"
assert engine_options["connect_args"] == {"check_same_thread": False}

def test_update_nonempty_connect_args(self):
engine_options = {"foo": "some foo", "connect_args": {"bar": "some bar"}}
updated = set_sqlite_connect_args(engine_options, SQLITE_URL)
assert len(updated) == 2
assert updated["foo"] == "some foo"
assert len(updated["connect_args"]) == 2
assert updated["connect_args"]["check_same_thread"] is False
assert updated["connect_args"]["bar"] == "some bar"
set_sqlite_connect_args(engine_options, SQLITE_URL)
assert len(engine_options) == 2
assert engine_options["foo"] == "some foo"
assert len(engine_options["connect_args"]) == 2
assert engine_options["connect_args"]["check_same_thread"] is False # type:ignore[index]
assert engine_options["connect_args"]["bar"] == "some bar" # type:ignore[index]

0 comments on commit b644b1d

Please sign in to comment.