Skip to content

Refactor LocalStore lock/transaction handling into context managers #396

Description

@peteski22

Refactor the repetitive lock and transaction handling in LocalStore (sdk/python/src/cq/store.py) into two small context managers. Internal cleanup only; no public API changes.

Proposed and discussed in #387.

Background

LocalStore repeats two patterns:

  • with self._lock: self._check_open() appears ~8 times across methods.
  • with self._conn: transaction wrapping appears in insert, delete, and update.

Proposed change

Extract two helpers:

  • _locked() — acquire self._lock and assert the store is open.
  • _transact() — compose _locked() with the with self._conn: transaction, yielding the connection.

self._lock is a non-reentrant threading.Lock, so _transact must acquire the lock exactly once (via _locked); add a comment noting the non-reentrancy so a future edit does not introduce a self-deadlock.

Scope

  • Limited to LocalStore in sdk/python/src/cq/store.py.
  • No changes to the public interface, the server backend, or Client.
  • Call sites should use the yielded handle (with self._transact() as conn:) rather than referencing self._conn inside the block, so the two styles do not drift.
  • Read methods (get, all, query, stats) may adopt _locked() but must keep their current semantics and stay outside any transaction.

Acceptance criteria

  • _locked() and _transact() added with the lock-discipline comment.
  • insert, delete, update use _transact(); read methods use _locked() where applicable.
  • Tests cover the normal path (commit + lock release) and the exception path (rollback + lock release, store still usable afterward) for the new context managers.
  • No public API changes; existing tests pass.

Metadata

Metadata

Assignees

No one assigned

    Type

    No fields configured for Task.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions