You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 dependencyasyncdefget_topology() ->Topology:
fromrackscope.apiimportappasapp_moduletopology=app_module.TOPOLOGYiftopologyisNone:
raiseHTTPException(status_code=503, detail="Topology not loaded yet")
returntopology
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:
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.
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. Centralisingthem here breaks the historical circular import between app.py anddependencies.py.Module-level mutation is intentional. apply_config() in app.py is thesingle writer; reads from anywhere are safe because reloads are gatedby _CONFIG_RELOAD_LOCK."""fromtypingimportOptional, Dict, Listfromrackscope.model.domainimportTopology, Catalog, ChecksLibrary, MetricsLibrary, TopologyIndexfromrackscope.model.configimportAppConfigfromrackscope.telemetry.plannerimportTelemetryPlannerTOPOLOGY: Optional[Topology] =NoneCATALOG: Optional[Catalog] =NoneCHECKS_LIBRARY: Optional[ChecksLibrary] =NoneMETRICS_LIBRARY: Optional[MetricsLibrary] =NoneAPP_CONFIG: Optional[AppConfig] =NonePLANNER: Optional[TelemetryPlanner] =NoneTOPOLOGY_INDEX: Optional[TopologyIndex] =NoneTARGETS_BY_CHECK: Optional[Dict[str, Dict[str, List[str]]]] =None
Update app.py:
# At the top, replace the inline declarations with:fromrackscope.apiimportstate# 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 importsTOPOLOGY=state.TOPOLOGY# ← BACKWARD-COMPAT: alias kept for legacy direct importsCATALOG=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.
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
refactor: extract
api/state.pyto break the circular import workaround independencies.pyand prepare plugin migration toDepends()Symptom
src/rackscope/api/dependencies.pyresolves every FastAPI dependency that reads global state (topology, catalog, app config, planner, ...) through a deferred import of theappmodule:Reason:
app.pyitself importsdependencies.pyindirectly through the routers, creating a circular import that Python resolves only because the innerfrom rackscope.api import app as app_moduleis deferred to call-time.Two problems with this:
Fragility: any restructuring of
app.pythat changes which module-level globals exist can silently break dependencies. The compiler does not check thatapp_module.TOPOLOGYexists; only runtime AttributeError catches it.Plugin-side anti-pattern: the bundled plugins (
simulator,slurm) follow the same pattern — they directly readapp_module.TOPOLOGY,app_module.APP_CONFIG, etc., bypassing FastAPI'sDepends()mechanism entirely. This makes the plugin routes untestable withTestClientwithout monkey-patchingrackscope.api.appat 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
After extraction
The circular dependency is broken because
state.pyis a leaf module — it imports nothing fromapp.pyordependencies.py.Proposed Fix — Two-Phase Strategy
Phase 1 — Introduce
state.py(this PR)Create
src/rackscope/api/state.py:Update
app.py:Keep
_CONFIG_RELOAD_LOCKinapp.py(it's a lock primitive, not state to share with dependencies).For backwards compatibility during the transition, keep re-exports in
app.py: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 TOPOLOGYwill get a staleNone. The right backward-compat shim is module-level descriptors or just a clear deprecation: document that direct imports of state globals fromapp.pyare deprecated and must usestate.pyinstead. List in the PR description the call sites that need updating.Update
dependencies.py: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:to:
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
make test)mypypasses (make typecheck)dependencies.pyare gone (grepfrom rackscope.api import app as app_moduleindependencies.pyreturns no results)state.pyis a leaf module:grep "from rackscope" src/rackscope/api/state.pyreturns onlyfrom rackscope.model.*andfrom rackscope.telemetry.*(noapi.app, noapi.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 callTestClientbehaviour for routers usingDepends()— they continue to receive the right topology / config / planner instancesget_*dependency function withstate.TOPOLOGY = None→ raises 503 with the documenteddetailfrom rackscope.api.app import TOPOLOGY(or any of the other state globals) is used outside ofapp.pyitself. Update them in the same PR.Impact and Severity
Breaking Changes
At the API contract level: none. All endpoints behave identically.
Internal contract changes:
app.py(e.g.,from rackscope.api.app import TOPOLOGY) will see a stale value. These call sites need to be updated tofrom rackscope.api import statethenstate.TOPOLOGY. Audit and fix in scope of this PR.app_module.TOPOLOGYstill 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_ARCHITECTURAL.md, also called out in finding MAIN-01 (plugins coupling to globals)simulatorandslurmplugin migrations toDepends()AuthProviderProtocol) — thestate.pyextraction 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
simulatorandslurmplugin routes toDepends()— Phase 2, separate issuesapply_config's module-level mutation with a dedicatedAppStateclass — possible v2 evolution; for now keep the change minimal