fix(cli): close bib read connections in comments extract; test store fixture (refs #615)
Fix round 1: the sealed-docket test file's _store() helper never closed its in-memory Store, and extract's file-backed lookup connection was never closed either — both leaked sqlite3 connections that surfaced as nondeterministic ResourceWarnings when GC'd during unrelated tests. Replace _store() with a store fixture that closes on teardown, and have extract() close the private read connection _build_bib_helpers opens for file-backed stores (the :memory: case still reuses store's own connection, left untouched). Also reworded the _build_bib_helpers docstring: attach_callback is None for --no-bib or --no-attach; the store is None only for --no-bib.
This commit is contained in:
@@ -40,9 +40,9 @@ def _build_bib_helpers(use_bib: bool, attach: bool):
|
|||||||
``attach_callback(comment_id, comment_dir) -> None`` runs on the main
|
``attach_callback(comment_id, comment_dir) -> None`` runs on the main
|
||||||
thread and registers a single ``"Comment text"`` note on the bib item
|
thread and registers a single ``"Comment text"`` note on the bib item
|
||||||
containing combined.md rendered to HTML. Skips items that already have
|
containing combined.md rendered to HTML. Skips items that already have
|
||||||
one. Both ``attach_callback`` and the store are ``None`` when
|
one. ``attach_callback`` is ``None`` for --no-bib or --no-attach; the
|
||||||
--no-bib or --no-attach (store is ``None`` only for --no-bib, so
|
store is ``None`` only for --no-bib, so ``extract`` can still read
|
||||||
``extract`` can still read seals with --no-attach).
|
seals with --no-attach.
|
||||||
"""
|
"""
|
||||||
if not use_bib:
|
if not use_bib:
|
||||||
return (lambda _cid: ""), None, None
|
return (lambda _cid: ""), None, None
|
||||||
@@ -53,11 +53,13 @@ def _build_bib_helpers(use_bib: bool, attach: bool):
|
|||||||
from bib import connect
|
from bib import connect
|
||||||
|
|
||||||
store = connect()
|
store = connect()
|
||||||
|
own_con = False
|
||||||
if store._db_path == ":memory:": # noqa: SLF001
|
if store._db_path == ":memory:": # noqa: SLF001
|
||||||
con = store._con() # noqa: SLF001 — tests only: a fresh connection would be an empty DB
|
con = store._con() # noqa: SLF001 — tests only: a fresh connection would be an empty DB
|
||||||
else:
|
else:
|
||||||
con = sqlite3.connect(store._db_path, check_same_thread=False) # noqa: SLF001
|
con = sqlite3.connect(store._db_path, check_same_thread=False) # noqa: SLF001
|
||||||
con.row_factory = sqlite3.Row
|
con.row_factory = sqlite3.Row
|
||||||
|
own_con = True
|
||||||
lock = threading.Lock()
|
lock = threading.Lock()
|
||||||
|
|
||||||
def _row(comment_id: str):
|
def _row(comment_id: str):
|
||||||
@@ -71,6 +73,12 @@ def _build_bib_helpers(use_bib: bool, attach: bool):
|
|||||||
row = _row(comment_id)
|
row = _row(comment_id)
|
||||||
return (row["abstract"] or "") if row else ""
|
return (row["abstract"] or "") if row else ""
|
||||||
|
|
||||||
|
# extract() closes this on our behalf when it opened a private
|
||||||
|
# read connection (the file-backed branch above) — the :memory:
|
||||||
|
# connection is store's own and must outlive this call.
|
||||||
|
if own_con:
|
||||||
|
body_lookup.close = con.close
|
||||||
|
|
||||||
if not attach:
|
if not attach:
|
||||||
return body_lookup, None, store
|
return body_lookup, None, store
|
||||||
|
|
||||||
@@ -153,18 +161,25 @@ def extract(
|
|||||||
raise typer.Exit(1)
|
raise typer.Exit(1)
|
||||||
|
|
||||||
lookup, attach_cb, store = _build_bib_helpers(use_bib, attach=use_bib and attach)
|
lookup, attach_cb, store = _build_bib_helpers(use_bib, attach=use_bib and attach)
|
||||||
skip = set(store.sealed_dockets()) if (store is not None and not force) else set()
|
try:
|
||||||
stats = walk_and_extract(
|
skip = (
|
||||||
root,
|
set(store.sealed_dockets()) if (store is not None and not force) else set()
|
||||||
inline_body_lookup=lookup,
|
)
|
||||||
docket=docket,
|
stats = walk_and_extract(
|
||||||
limit=limit or None,
|
root,
|
||||||
workers=workers,
|
inline_body_lookup=lookup,
|
||||||
force=force,
|
docket=docket,
|
||||||
on_extracted=attach_cb,
|
limit=limit or None,
|
||||||
skip_dockets=skip,
|
workers=workers,
|
||||||
reattach=reattach,
|
force=force,
|
||||||
)
|
on_extracted=attach_cb,
|
||||||
|
skip_dockets=skip,
|
||||||
|
reattach=reattach,
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
close = getattr(lookup, "close", None)
|
||||||
|
if close is not None:
|
||||||
|
close()
|
||||||
if docket and docket in skip:
|
if docket and docket in skip:
|
||||||
typer.echo(f" {docket}: sealed; skipped — use --force")
|
typer.echo(f" {docket}: sealed; skipped — use --force")
|
||||||
for k, v in stats.items():
|
for k, v in stats.items():
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ from pathlib import Path
|
|||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
import fitz
|
import fitz
|
||||||
|
import pytest
|
||||||
from typer.testing import CliRunner
|
from typer.testing import CliRunner
|
||||||
|
|
||||||
from bib.dockets import Docket
|
from bib.dockets import Docket
|
||||||
@@ -13,8 +14,13 @@ from cli.comments import app
|
|||||||
runner = CliRunner()
|
runner = CliRunner()
|
||||||
|
|
||||||
|
|
||||||
def _store() -> Store:
|
@pytest.fixture
|
||||||
return Store(":memory:", storage_dir="/tmp/nope")
|
def store() -> Store:
|
||||||
|
s = Store(":memory:", storage_dir="/tmp/nope")
|
||||||
|
try:
|
||||||
|
yield s
|
||||||
|
finally:
|
||||||
|
s.close()
|
||||||
|
|
||||||
|
|
||||||
def _seed_dir(root: Path, docket: str) -> Path:
|
def _seed_dir(root: Path, docket: str) -> Path:
|
||||||
@@ -28,8 +34,8 @@ def _seed_dir(root: Path, docket: str) -> Path:
|
|||||||
|
|
||||||
|
|
||||||
@patch("bib.connect")
|
@patch("bib.connect")
|
||||||
def test_dockets_table_lists_rows(mc_connect):
|
def test_dockets_table_lists_rows(mc_connect, store):
|
||||||
s = _store()
|
s = store
|
||||||
s.docket_upsert(
|
s.docket_upsert(
|
||||||
Docket(
|
Docket(
|
||||||
id="CMS-2019-0111",
|
id="CMS-2019-0111",
|
||||||
@@ -52,8 +58,8 @@ def test_dockets_table_lists_rows(mc_connect):
|
|||||||
|
|
||||||
|
|
||||||
@patch("bib.connect")
|
@patch("bib.connect")
|
||||||
def test_seal_and_unseal(mc_connect):
|
def test_seal_and_unseal(mc_connect, store):
|
||||||
s = _store()
|
s = store
|
||||||
s.docket_upsert(Docket(id="CMS-2019-0111"))
|
s.docket_upsert(Docket(id="CMS-2019-0111"))
|
||||||
mc_connect.return_value = s
|
mc_connect.return_value = s
|
||||||
assert (
|
assert (
|
||||||
@@ -68,8 +74,8 @@ def test_seal_and_unseal(mc_connect):
|
|||||||
|
|
||||||
|
|
||||||
@patch("bib.connect")
|
@patch("bib.connect")
|
||||||
def test_seal_unknown_docket_fails(mc_connect):
|
def test_seal_unknown_docket_fails(mc_connect, store):
|
||||||
mc_connect.return_value = _store()
|
mc_connect.return_value = store
|
||||||
result = runner.invoke(app, ["seal", "CMS-0000-0000"])
|
result = runner.invoke(app, ["seal", "CMS-0000-0000"])
|
||||||
assert result.exit_code == 1
|
assert result.exit_code == 1
|
||||||
assert "unknown docket" in result.output
|
assert "unknown docket" in result.output
|
||||||
@@ -78,10 +84,10 @@ def test_seal_unknown_docket_fails(mc_connect):
|
|||||||
@patch("bib.regulations_gov.discover_docket")
|
@patch("bib.regulations_gov.discover_docket")
|
||||||
@patch("bib.regulations_gov.Client")
|
@patch("bib.regulations_gov.Client")
|
||||||
@patch("bib.connect")
|
@patch("bib.connect")
|
||||||
def test_dockets_discover_populates_from_tags(mc_connect, mc_client, mc_disc):
|
def test_dockets_discover_populates_from_tags(mc_connect, mc_client, mc_disc, store):
|
||||||
from bib.item import Source
|
from bib.item import Source
|
||||||
|
|
||||||
s = _store()
|
s = store
|
||||||
it = Source(title="c", url="https://www.regulations.gov/comment/CMS-2019-0111-1")
|
it = Source(title="c", url="https://www.regulations.gov/comment/CMS-2019-0111-1")
|
||||||
it.add_tag("reg-docket:CMS-2019-0111")
|
it.add_tag("reg-docket:CMS-2019-0111")
|
||||||
it.add_tag("rule:CMS-1693-P")
|
it.add_tag("rule:CMS-1693-P")
|
||||||
@@ -106,8 +112,8 @@ def test_dockets_discover_populates_from_tags(mc_connect, mc_client, mc_disc):
|
|||||||
|
|
||||||
|
|
||||||
@patch("bib.connect")
|
@patch("bib.connect")
|
||||||
def test_extract_skips_sealed_docket(mc_connect, tmp_path: Path):
|
def test_extract_skips_sealed_docket(mc_connect, tmp_path: Path, store):
|
||||||
s = _store()
|
s = store
|
||||||
s.docket_upsert(
|
s.docket_upsert(
|
||||||
Docket(
|
Docket(
|
||||||
id="CMS-2024-0001", sealed_at="2025-01-01T00:00:00Z", seal_reason="manual"
|
id="CMS-2024-0001", sealed_at="2025-01-01T00:00:00Z", seal_reason="manual"
|
||||||
@@ -124,8 +130,8 @@ def test_extract_skips_sealed_docket(mc_connect, tmp_path: Path):
|
|||||||
|
|
||||||
|
|
||||||
@patch("bib.connect")
|
@patch("bib.connect")
|
||||||
def test_extract_force_runs_sealed_docket(mc_connect, tmp_path: Path):
|
def test_extract_force_runs_sealed_docket(mc_connect, tmp_path: Path, store):
|
||||||
s = _store()
|
s = store
|
||||||
s.docket_upsert(Docket(id="CMS-2024-0001", sealed_at="2025-01-01T00:00:00Z"))
|
s.docket_upsert(Docket(id="CMS-2024-0001", sealed_at="2025-01-01T00:00:00Z"))
|
||||||
mc_connect.return_value = s
|
mc_connect.return_value = s
|
||||||
cdir = _seed_dir(tmp_path, "CMS-2024-0001")
|
cdir = _seed_dir(tmp_path, "CMS-2024-0001")
|
||||||
|
|||||||
Reference in New Issue
Block a user