Skip to content

register_event_handlers has no app/session-factory access — handlers must build a parallel engine #90

Description

@antosubash

Summary

ModuleBase.register_event_handlers(self, bus: EventBus) -> None is the hook for subscribing to the framework's event bus. But the bus alone isn't enough — most useful event handlers need to persist something to the database in response to an event, and the handler has no way to reach the framework's existing engine / session factory.

Today's workaround: build a parallel async engine inside the handler module from Settings().database_url. That works but creates a second connection pool per worker process, and breaks subtly when the handler is invoked from a different event loop than the one the engine was created on (asyncpg connections are loop-bound).

Reproduction

Build a module whose only job is to materialize a row in response to events from another module — exactly what lacowiki_notifications does in laco_wiki_python's Phase 7. Inside register_event_handlers:

def register_event_handlers(self, bus: EventBus) -> None:
    bus.subscribe(SomeEvent, my_handler)

my_handler is async def my_handler(event) — it needs a session. There's no clean way to get one. The choices:

  1. Build a fresh engine per call. Costs one short-lived connection per event. Loop-safe (with engine.dispose() in finally).
  2. Build one engine per process and cache. Cheaper but breaks under cross-loop dispatch (eg a Celery task that uses asyncio.run from a fresh thread, the asyncpg connections are bound to the first loop).
  3. Reach into app.state.sm.db.session_factory somehow. No app reference is available at handler-call time.

Phase 7 ended up doing (1) after (2) caused a real test failure (the dispatcher's cached engine was poisoned across loops, causing the second event to silently fail; only the in-app row never landed).

What I'd hope for

Two complementary changes:

A. Pass app (or a context) into register_event_handlers

def register_event_handlers(self, bus: EventBus, app: FastAPI) -> None:
    factory = app.state.sm.db.session_factory  # captured in closure
    async def handler(event):
        async with factory() as session:
            ...
    bus.subscribe(SomeEvent, handler)

This matches how register_settings(app) already works. The hook is rarely used; changing the signature is a low-risk migration (modules that override it just add the app parameter).

B. Expose a worker-accessible session factory

For Celery tasks (sync), the framework already provides background_tasks.sync_db.sync_session(). The async-side analog could be:

from simple_module_db import async_session_factory

async with async_session_factory() as session:
    ...

…built once at boot and re-used. This would also help any handler that needs to do post-commit work outside the request session.

Why now

This same architecture wart shows up in two places in laco_wiki_python:

  1. Phase 7 dispatcher (lacowiki_notifications.services.dispatcher) — caches engine, then was forced to drop the cache and build per-call to survive cross-loop dispatch. See commit 1050020 in the project worktree.
  2. Worker tasks emitting events (lacowiki_reports.tasks.compute_report) — the worker has no bus reference at all, so it bypasses the bus entirely and calls the dispatcher's listener directly via asyncio.run. That's a workaround for the missing "how do worker tasks publish events?" story; the fix would be to make the bus accessible from sync workers (or to make the dispatcher importable as a plain async function, which it now is — but that path skips the bus's pub/sub indirection and any other subscribers).

Related framework gaps already filed

This issue compounds with:

Environment

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