Skip to content

6 - API (refactor): Remove Global State#231

Open
RobertRosca wants to merge 14 commits into
mainfrom
refactor/remove-global-state
Open

6 - API (refactor): Remove Global State#231
RobertRosca wants to merge 14 commits into
mainfrom
refactor/remove-global-state

Conversation

@RobertRosca

Copy link
Copy Markdown
Member

PR no. 6 of the big refactor.

This PR gets rid of the module-level singletons that the app bootstrapped at startup (_db.__ENGINE, _mymdc.CLIENT, auth.__CLIENT, TOKEN_STORE, registry metaclass, etc...) and replaces them with a single frozen AppState that's built once in the lifespan and handed out via DI. Also adds ADR-002 and ADR-003 which cover the reasoning.

Details

Previously any function could reach any dependency by importing a global, which is easy/short to write but has a bunch of downsides: init order is critical and pretty opaque, tests have to mutate/clear process-wide state between cases, you can't have two differently-configured apps in one process, in-process state looks shareable but silently breaks under multiple workers, etc...

ADR-002 goes through this and decides on a single typed AppState container built in the lifespan with dependencies injected everywhere else, so init is typed and order-explicit (defined by constructor) and tests inject fakes by building state instead of patching modules.

AppState + factories

state.py holds a frozen AppState dataclass and the create_* factory functions. The factories are pure - they take Settings (or already-built collaborators) and construct their object, no module state/side effects.

AppState currently has:

  • db_engine / db_sessionmaker
  • mymdc_client
  • oauth_client (None when auth is disabled)
  • token_store
  • damnit_registry
  • subscription_cursors

The lifespan builds all of these and attaches the result to app.state, slices can get it via a get_app_state FastAPI dependency.

Along the way this also:

  • swaps the TOKEN_STORE dict for a TokenStore (InMemoryTokenStore)
  • replaces the Registry metaclass with a plain DamnitDBRegistry
  • moves the subscription cursors off the module and onto AppState
  • drops some now-dead code (registry eviction, get_connection)

Settings

ADR-003 covers the way that settings are injected and is included here too since it is part of the global state that I wanted to remove, but (again) a bit of a lie since the actual removal of the settings = Settings() singleton isn't in this branch and create_app() still imports the global for now. The actual implementation of this is in a later branch.

Tests

test_state.py covers the AppState DI surface, factories, and stores. conftest.pys now build an AppState with fakes (mock MyMdC client, in-memory token store) instead of monkeypatching modules and clearing caches between tests 🎉

@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from ea9ed06 to 4ca4f6e Compare July 10, 2026 14:13
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch 2 times, most recently from 141c8aa to c90b2c7 Compare July 10, 2026 14:20
@RobertRosca RobertRosca changed the title API (refactor): Remove Global State 6 - API (refactor): Remove Global State Jul 10, 2026
@CammilleCC

Copy link
Copy Markdown
Member

Hmm, when I checked Settings() on the end of a stack, it still seems to be a singleton, and not injected. Could you please have a look if this is intended? (if it is, maybe we can relax the ADRs here?)

Also we still seem to use @alru_cache on the end, which is opposite to what the ADR2 is leading to. Maybe we can revisit these or the ADR as well.

@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 02a5f78 to c60b40f Compare July 17, 2026 06:20
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from c90b2c7 to afef36c Compare July 17, 2026 06:20
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from c60b40f to c80d085 Compare July 17, 2026 06:21
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from afef36c to 102d22a Compare July 17, 2026 06:21
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from c80d085 to 0ddd29c Compare July 17, 2026 08:40
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from 102d22a to 3dfd6d2 Compare July 17, 2026 08:40
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 0ddd29c to c9d9daf Compare July 17, 2026 09:09
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from 3dfd6d2 to 0873ce9 Compare July 17, 2026 09:09
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from c9d9daf to 4c5700c Compare July 17, 2026 09:11
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from 0873ce9 to dd5d976 Compare July 17, 2026 09:11
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 4c5700c to bf4af8b Compare July 17, 2026 09:12
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch 2 times, most recently from 1d7fdf0 to bd160c7 Compare July 17, 2026 09:12
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from bf4af8b to bce3ee7 Compare July 17, 2026 09:12
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from bd160c7 to 15f9109 Compare July 17, 2026 10:46
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch 2 times, most recently from 04f2042 to 951da6d Compare July 17, 2026 11:01
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch 2 times, most recently from 5c8c396 to 5db7605 Compare July 20, 2026 09:26
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch 2 times, most recently from 08d0c88 to 349cfa2 Compare July 20, 2026 09:28
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from 5db7605 to 4b537a3 Compare July 20, 2026 09:28
Base automatically changed from feat/improved-api-errors to main July 20, 2026 09:30
@RobertRosca
RobertRosca force-pushed the refactor/remove-global-state branch from 4b537a3 to 1d8be88 Compare July 20, 2026 09:30
@RobertRosca
RobertRosca requested a review from CammilleCC July 20, 2026 09:34
@RobertRosca RobertRosca self-assigned this Jul 20, 2026
@RobertRosca
RobertRosca marked this pull request as ready for review July 20, 2026 09:35
@CammilleCC

Copy link
Copy Markdown
Member

The question about Settings() still stands: you mentioned that it is intended to be removed, but I don't see it removed on the top of the stack. What are the plans for this?

Also on the @async_lru calls on MyMDC, there's a claim that we'll use Hishel instead of this. Do we still intend to do this? Of course we can leave the caches as these are now on the dependencies layer.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants