Skip to content

Encapsulate the _data_managers session registry #73

Description

@tschm

Subcategory: Overall architecture + Test design quality
Score: architecture 6 → ~8, test design 8 → 10

src/pycharting/api/interface.py:25 imports a private module global across module boundaries:

from pycharting.api.routes import _data_managers

and then mutates it at interface.py:221 (_data_managers[session_id] = data_manager) and reads it at :369 (len(_data_managers)).

Two problems:

  1. Encapsulation — the session registry is owned by routes.py (declared at routes.py:27) but written by interface.py, so neither module fully controls its lifecycle. The leading underscore signals "private" while being imported by name elsewhere.
  2. Thread safety — the dict is written from the caller's thread (plot() runs on the user's thread) and read by FastAPI request handlers on the server thread (routes.py:122,125,225,246,249) with no lock. Dict operations are individually atomic under the GIL, but the check-then-act sequences at routes.py:122/:246 (if session_id not in _data_managers followed by indexing/del) are not.

Suggested fix: give the registry an owning module with a public, locked accessor API (register_session, get_session, drop_session, session_count) and have both interface.py and routes.py use it. Then drop the ~51 test lines that reach into private attributes, which currently pin internals in place.

done when

_data_managers is referenced only inside its owning module, all cross-module access goes through public accessors, check-then-act sequences are guarded, and make test is green.

Evidence

src/pycharting/api/interface.py:25:from pycharting.api.routes import _data_managers
src/pycharting/api/interface.py:221:        _data_managers[session_id] = data_manager

Surfaced by the Rhiza quality gates during the template bump to jebel-quant/rhiza@v1.2.2, which enabled the stricter ruff rule families (ANN, A, BLE, ARG) and TYPECHECKER=both. Gate status at time of filing: make fmt FAIL, make typecheck FAIL, make docs-coverage FAIL, test-layout FAIL; make deptry, make security, make test PASS (154 tests, 100% coverage).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions