Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/codealmanac/services/index/schema.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from collections.abc import Iterator
from contextlib import contextmanager
from pathlib import Path

from codealmanac.database import (
Expand Down Expand Up @@ -130,3 +132,18 @@ def connect_index(path: Path) -> SQLiteConnection:
connection = connect_sqlite(path)
apply_migrations(connection, INDEX_MIGRATIONS)
return connection


@contextmanager
def open_index(path: Path) -> Iterator[SQLiteConnection]:
# `with connection:` scopes a transaction and never closes, so callers using
# it alone left the handle for the interpreter to reclaim instead of closing
# it deterministically. `open_local_database` exists for the same reason and
# records where that ended up once already. Nesting `with connection:` keeps
# the commit-on-success and rollback-on-error behaviour exactly as it was.
connection = connect_index(path)
try:
with connection:
yield connection
finally:
connection.close()
18 changes: 9 additions & 9 deletions src/codealmanac/services/index/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
stored_signature,
)
from codealmanac.services.index.requests import SearchIndexRequest
from codealmanac.services.index.schema import connect_index, index_db_path
from codealmanac.services.index.schema import index_db_path, open_index
from codealmanac.services.index.sources import load_index_sources
from codealmanac.services.index.views import (
build_health_report,
Expand All @@ -33,7 +33,7 @@ def refresh(self, almanac_path: Path, runtime_path: Path) -> IndexRefreshResult:
require_initialized_almanac_root(almanac_path)
sources = load_index_sources(almanac_path)
db_path = index_db_path(runtime_path)
with connect_index(db_path) as connection:
with open_index(db_path) as connection:
if stored_signature(connection) == sources.signature:
return IndexRefreshResult(
changed=0,
Expand All @@ -55,7 +55,7 @@ def rebuild(self, almanac_path: Path, runtime_path: Path) -> IndexRefreshResult:
require_initialized_almanac_root(almanac_path)
sources = load_index_sources(almanac_path)
db_path = index_db_path(runtime_path)
with connect_index(db_path) as connection:
with open_index(db_path) as connection:
replace_documents(connection, sources)
return IndexRefreshResult(
changed=len(sources.documents),
Expand All @@ -72,12 +72,12 @@ def search(
request: SearchIndexRequest,
) -> tuple[SearchPageResult, ...]:
require_initialized_almanac_root(almanac_path)
with connect_index(index_db_path(runtime_path)) as connection:
with open_index(index_db_path(runtime_path)) as connection:
return search_pages(connection, request)

def counts(self, almanac_path: Path, runtime_path: Path) -> IndexCounts:
require_initialized_almanac_root(almanac_path)
with connect_index(index_db_path(runtime_path)) as connection:
with open_index(index_db_path(runtime_path)) as connection:
return index_counts(connection)

def get_page(
Expand All @@ -87,7 +87,7 @@ def get_page(
slug: str,
) -> PageView | None:
require_initialized_almanac_root(almanac_path)
with connect_index(index_db_path(runtime_path)) as connection:
with open_index(index_db_path(runtime_path)) as connection:
return get_page_view(connection, slug)

def list_topics(
Expand All @@ -96,7 +96,7 @@ def list_topics(
runtime_path: Path,
) -> tuple[TopicSummary, ...]:
require_initialized_almanac_root(almanac_path)
with connect_index(index_db_path(runtime_path)) as connection:
with open_index(index_db_path(runtime_path)) as connection:
return list_topic_summaries(connection)

def get_topic(
Expand All @@ -107,7 +107,7 @@ def get_topic(
include_descendants: bool,
) -> TopicDetail | None:
require_initialized_almanac_root(almanac_path)
with connect_index(index_db_path(runtime_path)) as connection:
with open_index(index_db_path(runtime_path)) as connection:
return get_topic_detail(connection, slug, include_descendants)

def health_report(
Expand All @@ -118,7 +118,7 @@ def health_report(
registered_wikis: set[str],
) -> HealthReport:
require_initialized_almanac_root(almanac_path)
with connect_index(index_db_path(runtime_path)) as connection:
with open_index(index_db_path(runtime_path)) as connection:
return build_health_report(
connection,
repository_root,
Expand Down
36 changes: 36 additions & 0 deletions tests/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,39 @@ def tracking_connect(path: Path) -> sqlite3.Connection:
for connection in opened:
with pytest.raises(sqlite3.ProgrammingError):
connection.execute("SELECT 1")


def test_index_store_connections_close_after_every_use(tmp_path: Path, monkeypatch):
from codealmanac.services.index import schema
from codealmanac.services.index.store import IndexStore

almanac_path = tmp_path / "repo" / "almanac"
almanac_path.mkdir(parents=True)
(almanac_path / "README.md").write_text(
"---\ntopics: [concepts]\n---\n# Wiki\n\nRoot page.\n",
encoding="utf-8",
)
(almanac_path / "topics.yaml").write_text("topics: []\n", encoding="utf-8")
runtime_path = tmp_path / "runtime"

opened: list[sqlite3.Connection] = []
real_connect = schema.connect_sqlite

def tracking_connect(path: Path) -> sqlite3.Connection:
connection = real_connect(path)
opened.append(connection)
return connection

monkeypatch.setattr(schema, "connect_sqlite", tracking_connect)
store = IndexStore()

for _ in range(50):
store.refresh(almanac_path, runtime_path)

# Every query command reindexes implicitly, so this is the hot path rather
# than a rare one. IndexStore was the only store still relying on sqlite3's
# transaction-only context manager instead of closing.
assert len(opened) == 50
for connection in opened:
with pytest.raises(sqlite3.ProgrammingError):
connection.execute("SELECT 1")