mirror of
https://github.com/simonw/datasette.git
synced 2026-07-13 02:54:44 +02:00
Stop running sqlite-utils plugins on Datasette connections
Wrapping a connection in sqlite_utils.Database() runs sqlite-utils plugins' prepare_connection hooks against it by default. Datasette's write API views and introspection helpers now pass execute_plugins=False (matching what utils/internal_db.py already did), so third-party sqlite-utils plugins no longer touch Datasette's connections. Also apply PRAGMA recursive_triggers=on in Datasette._prepare_connection so every connection gets consistent trigger semantics - previously only the write connection got it, as a side effect of the first sqlite-utils based write. Refs #2831 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N76afGMhBRQk528VF1LTpR
This commit is contained in:
parent
a3f8b440e6
commit
5cec9c9faa
8 changed files with 75 additions and 8 deletions
|
|
@ -1565,6 +1565,10 @@ class Datasette:
|
|||
conn.execute("SELECT load_extension(?)", [extension])
|
||||
if self.setting("cache_size_kb"):
|
||||
conn.execute(f"PRAGMA cache_size=-{self.setting('cache_size_kb')}")
|
||||
# Consistent trigger semantics on every connection - sqlite-utils
|
||||
# would otherwise enable this on just the write connection, as a
|
||||
# side effect of the first sqlite-utils based write
|
||||
conn.execute("PRAGMA recursive_triggers=on")
|
||||
# pylint: disable=no-member
|
||||
if database != INTERNAL_DB_NAME:
|
||||
pm.hook.prepare_connection(conn=conn, database=database, datasette=self)
|
||||
|
|
|
|||
|
|
@ -757,7 +757,7 @@ class Database:
|
|||
|
||||
def column_details(conn):
|
||||
# Returns {column_name: (type, is_unique)}
|
||||
db = sqlite_utils.Database(conn)
|
||||
db = sqlite_utils.Database(conn, execute_plugins=False)
|
||||
columns = db[table].columns_dict
|
||||
indexes = db[table].indexes
|
||||
details = {}
|
||||
|
|
|
|||
|
|
@ -749,7 +749,9 @@ class RowDeleteView(BaseView):
|
|||
|
||||
# Delete table
|
||||
def delete_row(conn):
|
||||
sqlite_utils.Database(conn)[resolved.table].delete(resolved.pk_values)
|
||||
sqlite_utils.Database(conn, execute_plugins=False)[resolved.table].delete(
|
||||
resolved.pk_values
|
||||
)
|
||||
|
||||
try:
|
||||
await resolved.db.execute_write_fn(delete_row, request=request)
|
||||
|
|
@ -830,7 +832,7 @@ class RowUpdateView(BaseView):
|
|||
return Response.error(["Permission denied for alter-table"], 403)
|
||||
|
||||
def update_row(conn):
|
||||
sqlite_utils.Database(conn)[resolved.table].update(
|
||||
sqlite_utils.Database(conn, execute_plugins=False)[resolved.table].update(
|
||||
resolved.pk_values, update, alter=alter
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -1143,7 +1143,7 @@ class TableInsertView(BaseView):
|
|||
row_pk_values_for_later = [tuple(row[pk] for pk in pks) for row in rows]
|
||||
|
||||
def insert_or_upsert_rows(conn):
|
||||
table = sqlite_utils.Database(conn)[table_name]
|
||||
table = sqlite_utils.Database(conn, execute_plugins=False)[table_name]
|
||||
kwargs = {}
|
||||
if upsert:
|
||||
kwargs = {
|
||||
|
|
@ -1407,7 +1407,7 @@ class TableDropView(BaseView):
|
|||
|
||||
# Drop table
|
||||
def drop_table(conn):
|
||||
sqlite_utils.Database(conn)[table_name].drop()
|
||||
sqlite_utils.Database(conn, execute_plugins=False)[table_name].drop()
|
||||
|
||||
await db.execute_write_fn(drop_table, request=request)
|
||||
await self.ds.track_event(
|
||||
|
|
|
|||
|
|
@ -889,11 +889,13 @@ class TableCreateView(BaseView):
|
|||
initial_schema = None
|
||||
if table_exists:
|
||||
initial_schema = await db.execute_fn(
|
||||
lambda conn: sqlite_utils.Database(conn)[table_name].schema
|
||||
lambda conn: sqlite_utils.Database(conn, execute_plugins=False)[
|
||||
table_name
|
||||
].schema
|
||||
)
|
||||
|
||||
def create_table(conn):
|
||||
db_for_write = sqlite_utils.Database(conn)
|
||||
db_for_write = sqlite_utils.Database(conn, execute_plugins=False)
|
||||
table = db_for_write[table_name]
|
||||
if rows:
|
||||
table.insert_all(
|
||||
|
|
@ -1187,7 +1189,9 @@ class TableAlterView(BaseView):
|
|||
before_schema = _table_schema_from_conn(conn, table_name)
|
||||
|
||||
def apply_operations(operation_conn):
|
||||
db_for_write = sqlite_utils.Database(operation_conn)
|
||||
db_for_write = sqlite_utils.Database(
|
||||
operation_conn, execute_plugins=False
|
||||
)
|
||||
table = db_for_write[table_name]
|
||||
current_table_name = table_name
|
||||
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ Unreleased
|
|||
- ``await db.execute_write_script()`` is now transactional, matching its documentation: if any statement in the script fails, none of its statements are applied. Scripts containing statements that cannot run inside a transaction, or that manage transactions themselves, fall back to the previous ``conn.executescript()`` autocommit behavior. (:issue:`2831`)
|
||||
- The JSON write API is now atomic per request: ``/db/-/create`` with initial rows, multi-operation ``/db/table/-/alter`` calls and inserts using ``"return": true`` now either fully apply or roll back entirely if any part fails. Previously a failure part way through could leave earlier writes from the same request permanently committed. (:issue:`2831`)
|
||||
- Rebuilding the internal database catalog for a database is now a single atomic write. Previously the rebuild used six separate transactions, so queries against the internal database could observe a database with missing catalog rows while a rebuild was in progress. (:issue:`2831`)
|
||||
- sqlite-utils plugins no longer have their ``prepare_connection()`` hooks executed against Datasette's database connections - use Datasette's own :ref:`prepare_connection() <plugin_hook_prepare_connection>` plugin hook to customize connections. ``PRAGMA recursive_triggers=on`` is now applied consistently to every connection Datasette opens - previously it was enabled just on the write connection, as a side effect of the first sqlite-utils based write. (:issue:`2831`)
|
||||
|
||||
.. _v1_0_a36:
|
||||
|
||||
|
|
|
|||
|
|
@ -2782,3 +2782,38 @@ async def test_insert_with_return_failing_row_is_atomic(ds_write):
|
|||
await ds_write.get_database("data").execute("select count(*) from docs")
|
||||
).single_value()
|
||||
assert count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_write_api_does_not_run_sqlite_utils_plugins(ds_write):
|
||||
# https://github.com/simonw/datasette/issues/2831
|
||||
# sqlite-utils plugins should not have their prepare_connection hooks
|
||||
# executed against Datasette's connections
|
||||
import sqlite_utils.plugins
|
||||
from sqlite_utils import hookimpl
|
||||
|
||||
prepared = []
|
||||
|
||||
class TrackingPlugin:
|
||||
@hookimpl
|
||||
def prepare_connection(self, conn):
|
||||
prepared.append(conn)
|
||||
|
||||
sqlite_utils.plugins.pm.register(TrackingPlugin(), name="datasette-test-tracking")
|
||||
try:
|
||||
token = write_token(ds_write)
|
||||
response = await ds_write.client.post(
|
||||
"/data/docs/-/insert",
|
||||
json={"rows": [{"id": 1, "title": "one"}]},
|
||||
headers=_headers(token),
|
||||
)
|
||||
assert response.status_code == 201
|
||||
response = await ds_write.client.post(
|
||||
"/data/docs/1/-/update",
|
||||
json={"update": {"title": "two"}},
|
||||
headers=_headers(token),
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert prepared == []
|
||||
finally:
|
||||
sqlite_utils.plugins.pm.unregister(name="datasette-test-tracking")
|
||||
|
|
|
|||
|
|
@ -1302,3 +1302,24 @@ async def test_database_close_is_idempotent(tmpdir):
|
|||
# Second call should be a no-op, not raise
|
||||
db.close()
|
||||
ds._internal_database.close()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_recursive_triggers_enabled_on_all_connections(tmp_path):
|
||||
# https://github.com/simonw/datasette/issues/2831
|
||||
# Previously recursive_triggers was only enabled on the write connection,
|
||||
# and only as a side effect of the first sqlite-utils based write - so
|
||||
# trigger semantics could differ between connections
|
||||
path = str(tmp_path / "test.db")
|
||||
sqlite3.connect(path).close()
|
||||
datasette = Datasette([path])
|
||||
db = datasette.get_database("test")
|
||||
write_value = await db.execute_write_fn(
|
||||
lambda conn: conn.execute("PRAGMA recursive_triggers").fetchone()[0],
|
||||
transaction=False,
|
||||
)
|
||||
read_value = await db.execute_fn(
|
||||
lambda conn: conn.execute("PRAGMA recursive_triggers").fetchone()[0]
|
||||
)
|
||||
assert write_value == 1
|
||||
assert read_value == 1
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue