Skip to content

refactor: extract api/state.py to break circular import workaround and prepare plugin migration to Depends() #40

Description

@SckyzO

refactor: extract api/state.py to break the circular import workaround in dependencies.py and prepare plugin migration to Depends()

Symptom

src/rackscope/api/dependencies.py resolves every FastAPI dependency that reads global state (topology, catalog, app config, planner, ...) through a deferred import of the app module:

# src/rackscope/api/dependencies.py — repeated for every dependency
async def get_topology() -> Topology:
    from rackscope.api import app as app_module
    topology = app_module.TOPOLOGY
    if topology is None:
        raise HTTPException(status_code=503, detail="Topology not loaded yet")
    return topology

Reason: app.py itself imports dependencies.py indirectly through the routers, creating a circular import that Python resolves only because the inner from rackscope.api import app as app_module is deferred to call-time.

Two problems with this:

  1. Fragility: any restructuring of app.py that changes which module-level globals exist can silently break dependencies. The compiler does not check that app_module.TOPOLOGY exists; only runtime AttributeError catches it.

  2. Plugin-side anti-pattern: the bundled plugins (simulator, slurm) follow the same pattern — they directly read app_module.TOPOLOGY, app_module.APP_CONFIG, etc., bypassing FastAPI's Depends() mechanism entirely. This makes the plugin routes untestable with TestClient without monkey-patching rackscope.api.app at module level.

This is the architectural debt flagged as ARCH-01 in the audit. The recommended fix is the two-phase strategy explicitly prescribed by PROFILE_DEV.md: introduce the new mechanism in parallel, then migrate consumers.

Technical Analysis

Current dependency chain

app.py (defines TOPOLOGY, CATALOG, APP_CONFIG, PLANNER, ...)
  ↓ imports routers
  ↓
routers/*.py
  ↓ uses
  ↓
dependencies.py
  ↓ needs to read app.py globals
  ↓
  ⟲ would re-import app.py → circular
  ↓
  ↓ workaround: defer the import to call time
  └─ `from rackscope.api import app as app_module` (inside every getter)

After extraction

state.py (holds TOPOLOGY, CATALOG, APP_CONFIG, PLANNER, ... as module-level vars)
  ↑              ↑
  │              │
app.py       dependencies.py
  │              │
  │              └─→ reads from state.py directly, no deferred import
  │
  └─→ writes to state.py during apply_config()

The circular dependency is broken because state.py is a leaf module — it imports nothing from app.py or dependencies.py.

Proposed Fix — Two-Phase Strategy

Phase 1 — Introduce state.py (this PR)

Create src/rackscope/api/state.py:

"""Module-level state container for the FastAPI app.

This module holds the runtime globals that are mutated by apply_config()
and read by the FastAPI Depends() functions in dependencies.py. Centralising
them here breaks the historical circular import between app.py and
dependencies.py.

Module-level mutation is intentional. apply_config() in app.py is the
single writer; reads from anywhere are safe because reloads are gated
by _CONFIG_RELOAD_LOCK.
"""
from typing import Optional, Dict, List
from rackscope.model.domain import Topology, Catalog, ChecksLibrary, MetricsLibrary, TopologyIndex
from rackscope.model.config import AppConfig
from rackscope.telemetry.planner import TelemetryPlanner

TOPOLOGY: Optional[Topology] = None
CATALOG: Optional[Catalog] = None
CHECKS_LIBRARY: Optional[ChecksLibrary] = None
METRICS_LIBRARY: Optional[MetricsLibrary] = None
APP_CONFIG: Optional[AppConfig] = None
PLANNER: Optional[TelemetryPlanner] = None
TOPOLOGY_INDEX: Optional[TopologyIndex] = None
TARGETS_BY_CHECK: Optional[Dict[str, Dict[str, List[str]]]] = None

Update app.py:

# At the top, replace the inline declarations with:
from rackscope.api import state

# Inside apply_config / _do_apply_config, replace:
#   global TOPOLOGY, CATALOG, ...
#   TOPOLOGY = ...
# with:
#   state.TOPOLOGY = ...
#   state.CATALOG = ...

Keep _CONFIG_RELOAD_LOCK in app.py (it's a lock primitive, not state to share with dependencies).

For backwards compatibility during the transition, keep re-exports in app.py:

# In app.py, after the imports
TOPOLOGY = state.TOPOLOGY  # ← BACKWARD-COMPAT: alias kept for legacy direct imports
CATALOG = state.CATALOG
# etc.

Note: Python module-level variables are not "live aliases" — they capture the value at import time. The re-exports above are misleading. The actual mechanism is: existing callers that do from rackscope.api.app import TOPOLOGY will get a stale None. The right backward-compat shim is module-level descriptors or just a clear deprecation: document that direct imports of state globals from app.py are deprecated and must use state.py instead. List in the PR description the call sites that need updating.

Update dependencies.py:

# Replace
async def get_topology() -> Topology:
    from rackscope.api import app as app_module
    topology = app_module.TOPOLOGY
    ...

# With
from rackscope.api import state

async def get_topology() -> Topology:
    if state.TOPOLOGY is None:
        raise HTTPException(status_code=503, detail="Topology not loaded yet")
    return state.TOPOLOGY

Repeat for every get_* dependency. The deferred import disappears.

Phase 2 — Migrate plugin code paths to Depends() (separate follow-up PR)

Once Phase 1 is stable, plugins (simulator, slurm) migrate from:

# plugins/slurm/backend/plugin.py
APP_CONFIG = app_module.APP_CONFIG
TOPOLOGY = app_module.TOPOLOGY

to:

@router.get("/api/slurm/jobs")
async def list_jobs(
    app_config: Annotated[AppConfig, Depends(get_app_config)],
    topology: Annotated[Topology, Depends(get_topology)],
):
    ...

Each plugin migration is its own small PR. Both bundled plugins should be migrated before declaring the old direct-access pattern unsupported.

This issue covers Phase 1 only. Phase 2 is tracked as separate follow-up issues per plugin.

Test Checklist

  • Full existing test suite passes unchanged (make test)
  • mypy passes (make typecheck)
  • All deferred imports in dependencies.py are gone (grep from rackscope.api import app as app_module in dependencies.py returns no results)
  • state.py is a leaf module: grep "from rackscope" src/rackscope/api/state.py returns only from rackscope.model.* and from rackscope.telemetry.* (no api.app, no api.dependencies)
  • apply_config() and _do_apply_config() still work end-to-end — there is at least one integration test that triggers a config reload and verifies the new state is visible to a follow-up API call
  • No regression on the TestClient behaviour for routers using Depends() — they continue to receive the right topology / config / planner instances
  • New test: directly invoke a get_* dependency function with state.TOPOLOGY = None → raises 503 with the documented detail
  • Document in the PR description the list of files where from rackscope.api.app import TOPOLOGY (or any of the other state globals) is used outside of app.py itself. Update them in the same PR.

Impact and Severity

  • Audience affected: contributors. No user-visible change.
  • Severity: medium (architectural debt). Not breaking anything today, but the workaround grows more fragile with every restructuring. The plugins-using-direct-access pattern blocks testability of plugin routes — a real coding tax.
  • Priority: Sprint 4 of the audit roadmap. Ships when convenient, no urgency. Good candidate for a refactor-themed PR with extra review time.

Breaking Changes

At the API contract level: none. All endpoints behave identically.

Internal contract changes:

  • Callers that import state globals directly from app.py (e.g., from rackscope.api.app import TOPOLOGY) will see a stale value. These call sites need to be updated to from rackscope.api import state then state.TOPOLOGY. Audit and fix in scope of this PR.
  • Plugins continue to work in Phase 1 (their direct access via app_module.TOPOLOGY still resolves to the re-exported alias, but stale). Plugin migration (Phase 2) makes the direct access mechanism go away. Each plugin migration is non-breaking on its own.

Related

  • Audit ref: ARCH-01 in AUDIT_ARCHITECTURAL.md, also called out in finding MAIN-01 (plugins coupling to globals)
  • Phase 2 follow-up: separate issues for simulator and slurm plugin migrations to Depends()
  • The RFC RBAC (Issue RBAC v1 — Phase 1: local-only authentication & authorization #25 / Phase 2) anticipates a similar abstraction (AuthProvider Protocol) — the state.py extraction does not block or conflict with that future work, but provides a cleaner module layout that the auth refactor can build on.

Out of Scope

  • Migrating simulator and slurm plugin routes to Depends() — Phase 2, separate issues
  • Introducing a generic "service registry" abstraction — out of scope, would be over-engineering for a v1 codebase
  • Replacing apply_config's module-level mutation with a dedicated AppState class — possible v2 evolution; for now keep the change minimal

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions