Skip to content

Commit d15c2de

Browse files
authored
Merge pull request #101 from modern-python/refactor/ref-6-frozen-setattr
refactor: drop frozen=True from instruments; sentinel for FastAPIConfig.application
2 parents 7a15f93 + 153868c commit d15c2de

17 files changed

Lines changed: 119 additions & 109 deletions

‎docs/superpowers/plans/2026-06-01-pr13-frozen-setattr.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
- **REF-6**: Drop `frozen=True` from the instrument hierarchy. Python's dataclass rules force a cascade: dropping `frozen=True` from `LoggingInstrument` requires dropping it from `BaseInstrument`, which requires dropping it from every other instrument subclass. 23 dataclass declarations across 12 files lose `frozen=True`. In `LoggingInstrument` and `OpenTelemetryInstrument`, the four `object.__setattr__(self, "_x", value)` workarounds for cached runtime state become plain `self._x = value` assignments. **Configs stay frozen** — only instruments lose `frozen=True`.
88

9-
- **LOW-4**: `FastAPIConfig.application` currently uses `default=None` + `# ty: ignore[invalid-assignment]` because the field is typed `fastapi.FastAPI` (non-Optional). Replace with a typed-sentinel pattern: `_UNSET_FASTAPI_APP: typing.Final = typing.cast("fastapi.FastAPI", object())`. The `__post_init__` checks `self.application is _UNSET_FASTAPI_APP` instead of `not self.application`. Drops the `# ty: ignore`. FastAPIConfig stays frozen (so the existing `object.__setattr__(self, "application", ...)` in `__post_init__` remains — only the default-and-sentinel-check changes).
9+
- **LOW-4**: `FastAPIConfig.application` currently uses `default=None` + `# ty: ignore[invalid-assignment]` because the field is typed `fastapi.FastAPI` (non-Optional). Replace with a proper sentinel-type pattern: introduce `UnsetType` + `UNSET` in `lite_bootstrap/types.py` (a sentinel class with a singleton instance), type the field as `fastapi.FastAPI | UnsetType`, default to `UNSET`, and replace the truthiness check in `__post_init__` with `isinstance(self.application, UnsetType)`. Add a `_narrow_app(config)` helper at module scope that asserts the type and returns the narrowed value; FastAPI framework instruments call `_narrow_app(self.bootstrap_config)` instead of `self.bootstrap_config.application` directly. Drops the `# ty: ignore`. FastAPIConfig stays frozen — `object.__setattr__(self, "application", ...)` in `__post_init__` remains because the freeze bypass is the only way to mutate a frozen field after construction; a code comment documents the rationale.
1010

1111
**Architecture:** Largest mechanical refactor in the deferred-refactors sequence. The cascade is purely mechanical — every change is `frozen=True` → (delete). The setattr replacements and LOW-4 sentinel are the only meaningful diffs.
1212

@@ -48,16 +48,20 @@ The sequencing spec's PR13 section said "drop `frozen=True` from `LoggingInstrum
4848
- `lite_bootstrap/instruments/swagger_instrument.py` — `SwaggerInstrument` loses `frozen=True`.
4949

5050
**Bootstrapper modules (3 files):**
51-
- `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py` — 5 framework instruments lose `frozen=True` + LOW-4 sentinel pattern on `FastAPIConfig.application`.
51+
- `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py` — 5 framework instruments lose `frozen=True` + LOW-4 sentinel-type pattern on `FastAPIConfig.application` + `_narrow_app` helper + every FastAPI instrument bootstrap calls `_narrow_app(self.bootstrap_config)` for the app reference.
5252
- `lite_bootstrap/bootstrappers/litestar_bootstrapper.py` — 6 framework instruments lose `frozen=True`.
5353
- `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` — 4 framework instruments lose `frozen=True`.
5454

55+
**Shared types (1 file):**
56+
- `lite_bootstrap/types.py` — add `UnsetType` class + `UNSET: typing.Final[UnsetType]` singleton. Reusable sentinel for fields that distinguish "not passed" from "explicitly None".
57+
5558
---
5659

5760
## Locked decisions
5861

5962
- **Cascade scope:** Drop `frozen=True` from `BaseInstrument` and ALL 22 instrument subclasses. Configs stay frozen. Confirmed by the user after the constraint surfaced.
60-
- **LOW-4 pattern:** Typed sentinel via `typing.cast`. Cleaner than the current `# ty: ignore` once the cast obscures the type lie.
63+
- **LOW-4 pattern:** Proper `UnsetType` sentinel class in `lite_bootstrap/types.py`, used via `isinstance(value, UnsetType)`. Honest to the type checker (no `typing.cast` lie). Adds a `_narrow_app` helper that wraps the assert/return narrowing for callers. Revised from the original plan's `typing.cast("fastapi.FastAPI", object())` pattern — the spec was updated retroactively to match what was built.
64+
- **`FastAPIConfig` stays frozen:** Confirmed by user. The `object.__setattr__(self, "application", ...)` in `__post_init__` remains; a one-line code comment documents the rationale (frozen for user-facing immutability; bypass needed because `application` is constructed using other config fields).
6165
- **No new tests.** The full existing test suite verifies behavior preservation; pure-refactor changes should not affect runtime semantics other than enabling future direct mutation (which we don't exercise).
6266

6367
---

‎docs/superpowers/specs/2026-06-01-deferred-refactors-sequencing.md‎

Lines changed: 33 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -211,60 +211,42 @@ suite passes against the new layout.
211211

212212
**Scope:**
213213

214-
- REF-6: Drop `frozen=True` from `LoggingInstrument` and `OpenTelemetryInstrument`
215-
(the only two instruments that legitimately cache mutable runtime state via
216-
`object.__setattr__`). Replace `object.__setattr__(self, "_x", value)` with plain
217-
`self._x = value`. The exception-safety from PR3's `try/finally` shape stays
218-
intact — the pattern simplifies from:
219-
220-
```python
221-
if self._tracer_provider is not None:
222-
try:
223-
self._tracer_provider.shutdown()
224-
finally:
225-
object.__setattr__(self, "_tracer_provider", None)
226-
```
227-
228-
to:
229-
230-
```python
231-
if self._tracer_provider is not None:
232-
try:
233-
self._tracer_provider.shutdown()
234-
finally:
235-
self._tracer_provider = None
236-
```
237-
238-
Configs stay frozen — only the two instruments with cached state lose `frozen`.
239-
Apply to both `LoggingInstrument._logger_factory` resets and
240-
`OpenTelemetryInstrument._tracer_provider` resets, including in their bootstrap
241-
paths.
242-
243-
- LOW-4: Address `FastAPIConfig.application` declared with
244-
`default=None` + `# ty: ignore[invalid-assignment]` and patched in `__post_init__`.
245-
The minimum-risk cleanup is to use a sentinel instead of `None`:
246-
247-
```python
248-
_UNSET: typing.Final = typing.cast("fastapi.FastAPI", object())
249-
application: "fastapi.FastAPI" = _UNSET
250-
```
251-
252-
Drop the `# ty: ignore`. The `__post_init__` check changes from `if not
253-
self.application:` to `if self.application is _UNSET:`. Litestar's
254-
`default_factory=lambda: AppConfig()` pattern doesn't work here because the
255-
factory needs `service_name` from the config to set `app.title` etc.
256-
257-
**Files:** 3-4 — `logging_instrument.py`, `opentelemetry_instrument.py`,
258-
`fastapi_bootstrapper.py`. Possibly `tests/instruments/test_*_instrument.py` if any
259-
tests rely on instrument immutability (`dataclasses.replace`, `frozen` errors).
214+
- REF-6: Python's dataclass rules forbid surgical unfreezing (a non-frozen
215+
dataclass can't inherit from a frozen one). To drop `frozen=True` from
216+
`LoggingInstrument` and `OpenTelemetryInstrument` (the two instruments with
217+
`object.__setattr__` workarounds), `BaseInstrument` and all 22 instrument
218+
subclasses must also lose `frozen=True`. Configs all keep `frozen=True`.
219+
After the cascade, 4 `object.__setattr__(self, "_x", value)` call sites in
220+
the two instruments become plain `self._x = value`. The `try/finally`
221+
exception safety from PR3 is preserved.
222+
223+
- LOW-4: `FastAPIConfig.application` declared with `default=None` +
224+
`# ty: ignore[invalid-assignment]`. Replaced with a proper sentinel-type
225+
pattern: introduce `UnsetType` + `UNSET` singleton in
226+
`lite_bootstrap/types.py`, type the field as `fastapi.FastAPI | UnsetType`,
227+
default to `UNSET`, check via `isinstance(self.application, UnsetType)`. Add
228+
a `_narrow_app(config)` helper that asserts the type and returns the
229+
narrowed app; every FastAPI framework instrument calls it. Drops the
230+
`# ty: ignore`. `FastAPIConfig` stays frozen — the existing
231+
`object.__setattr__(self, "application", ...)` in `__post_init__` remains
232+
(a code comment documents the rationale). Sibling configs (`LitestarConfig`,
233+
`FastStreamConfig`) don't have this need because they use `default_factory`
234+
for their app fields.
235+
236+
**Note:** the originally-planned `typing.cast("fastapi.FastAPI", object())`
237+
sentinel was replaced during implementation with a proper `UnsetType` class.
238+
This spec has been retroactively updated to match what was built.
239+
240+
**Files:** 13 — 9 instrument modules (`base.py` + 8 base instruments), 3
241+
bootstrapper modules (`fastapi`, `litestar`, `faststream`), and `types.py`
242+
(new `UnsetType` + `UNSET` sentinel).
260243

261244
**Test impact:** Existing tests should pass unchanged. Watch for any test that relied
262-
on `FrozenInstanceError` being raised on instrument mutation — none expected, but
263-
verify.
245+
on `FrozenInstanceError` being raised on instrument mutation — none expected.
264246

265-
**Risk:** Medium. The `frozen` change is observable to user code that relied on
266-
`dataclasses.replace` for `LoggingInstrument` / `OpenTelemetryInstrument`. Unlikely
267-
in practice but worth noting.
247+
**Risk:** Medium. The cascade is mechanical but missing one entry breaks the build
248+
(TypeError at import). The `frozen` change is observable to user code that relied on
249+
`dataclasses.replace` for instruments — unlikely in practice but worth noting.
268250

269251
---
270252

‎lite_bootstrap/bootstrappers/fastapi_bootstrapper.py‎

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from lite_bootstrap.instruments.pyroscope_instrument import PyroscopeConfig, PyroscopeInstrument
2020
from lite_bootstrap.instruments.sentry_instrument import SentryConfig, SentryInstrument
2121
from lite_bootstrap.instruments.swagger_instrument import SwaggerConfig, SwaggerInstrument
22+
from lite_bootstrap.types import UNSET, UnsetType
2223

2324

2425
if import_checker.is_fastapi_installed:
@@ -47,7 +48,7 @@ class FastAPIConfig(
4748
SentryConfig,
4849
SwaggerConfig,
4950
):
50-
application: "fastapi.FastAPI" = dataclasses.field(default=None) # ty: ignore[invalid-assignment]
51+
application: "fastapi.FastAPI | UnsetType" = UNSET
5152
application_kwargs: dict[str, typing.Any] = dataclasses.field(default_factory=dict)
5253
opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list)
5354
prometheus_instrumentator_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict)
@@ -59,24 +60,32 @@ def __post_init__(self) -> None:
5960
msg = "fastapi is not installed"
6061
raise ConfigurationError(msg)
6162

62-
if not self.application:
63-
object.__setattr__(
64-
self, "application", fastapi.FastAPI(docs_url=self.swagger_path, **self.application_kwargs)
65-
)
66-
elif self.application_kwargs:
67-
warnings.warn("application_kwargs must be used without application", stacklevel=2)
63+
if isinstance(self.application, UnsetType):
64+
application = fastapi.FastAPI(docs_url=self.swagger_path, **self.application_kwargs)
65+
# FastAPIConfig stays frozen for user-facing immutability; __post_init__ needs
66+
# to set application after construction, so we bypass the freeze here.
67+
object.__setattr__(self, "application", application)
68+
else:
69+
application = self.application
70+
if self.application_kwargs:
71+
warnings.warn("application_kwargs must be used without application", stacklevel=2)
6872

69-
self.application.title = self.service_name
70-
self.application.debug = self.service_debug
71-
self.application.version = self.service_version
73+
application.title = self.service_name
74+
application.debug = self.service_debug
75+
application.version = self.service_version
7276

7377

74-
@dataclasses.dataclass(kw_only=True, slots=True, frozen=True)
78+
def _narrow_app(config: "FastAPIConfig") -> "fastapi.FastAPI":
79+
assert not isinstance(config.application, UnsetType)
80+
return config.application
81+
82+
83+
@dataclasses.dataclass(kw_only=True, slots=True)
7584
class FastAPICorsInstrument(CorsInstrument):
7685
bootstrap_config: FastAPIConfig
7786

7887
def bootstrap(self) -> None:
79-
self.bootstrap_config.application.add_middleware(
88+
_narrow_app(self.bootstrap_config).add_middleware(
8089
CORSMiddleware,
8190
allow_origins=self.bootstrap_config.cors_allowed_origins,
8291
allow_methods=self.bootstrap_config.cors_allowed_methods,
@@ -88,7 +97,7 @@ def bootstrap(self) -> None:
8897
)
8998

9099

91-
@dataclasses.dataclass(kw_only=True, slots=True, frozen=True)
100+
@dataclasses.dataclass(kw_only=True, slots=True)
92101
class FastAPIHealthChecksInstrument(HealthChecksInstrument):
93102
bootstrap_config: FastAPIConfig
94103

@@ -105,27 +114,27 @@ async def health_check_handler() -> HealthCheckTypedDict:
105114
return fastapi_router
106115

107116
def bootstrap(self) -> None:
108-
self.bootstrap_config.application.include_router(self.build_fastapi_health_check_router())
117+
_narrow_app(self.bootstrap_config).include_router(self.build_fastapi_health_check_router())
109118

110119

111-
@dataclasses.dataclass(kw_only=True, frozen=True)
120+
@dataclasses.dataclass(kw_only=True)
112121
class FastAPIOpenTelemetryInstrument(OpenTelemetryInstrument):
113122
bootstrap_config: FastAPIConfig
114123

115124
def bootstrap(self) -> None:
116125
super().bootstrap()
117126
FastAPIInstrumentor.instrument_app(
118-
app=self.bootstrap_config.application,
127+
app=_narrow_app(self.bootstrap_config),
119128
tracer_provider=get_tracer_provider(),
120129
excluded_urls=",".join(self._build_excluded_urls()),
121130
)
122131

123132
def teardown(self) -> None:
124-
FastAPIInstrumentor.uninstrument_app(self.bootstrap_config.application)
133+
FastAPIInstrumentor.uninstrument_app(_narrow_app(self.bootstrap_config))
125134
super().teardown()
126135

127136

128-
@dataclasses.dataclass(kw_only=True, frozen=True)
137+
@dataclasses.dataclass(kw_only=True)
129138
class FastAPIPrometheusInstrument(PrometheusInstrument):
130139
bootstrap_config: FastAPIConfig
131140
missing_dependency_message = "prometheus_fastapi_instrumentator is not installed"
@@ -135,32 +144,31 @@ def check_dependencies() -> bool:
135144
return import_checker.is_prometheus_fastapi_instrumentator_installed
136145

137146
def bootstrap(self) -> None:
147+
application = _narrow_app(self.bootstrap_config)
138148
Instrumentator(**self.bootstrap_config.prometheus_instrumentator_params).instrument(
139-
self.bootstrap_config.application,
149+
application,
140150
**self.bootstrap_config.prometheus_instrument_params,
141151
).expose(
142-
self.bootstrap_config.application,
152+
application,
143153
endpoint=self.bootstrap_config.prometheus_metrics_path,
144154
include_in_schema=self.bootstrap_config.prometheus_metrics_include_in_schema,
145155
**self.bootstrap_config.prometheus_expose_params,
146156
)
147157

148158

149-
@dataclasses.dataclass(kw_only=True, frozen=True)
159+
@dataclasses.dataclass(kw_only=True)
150160
class FastAPISwaggerInstrument(SwaggerInstrument):
151161
bootstrap_config: FastAPIConfig
152162

153163
def bootstrap(self) -> None:
154-
if self.bootstrap_config.swagger_path != self.bootstrap_config.application.docs_url:
164+
application = _narrow_app(self.bootstrap_config)
165+
if self.bootstrap_config.swagger_path != application.docs_url:
155166
warnings.warn(
156-
f"swagger_path differs from docs_url, "
157-
f"{self.bootstrap_config.application.docs_url} will be used for docs path",
167+
f"swagger_path differs from docs_url, {application.docs_url} will be used for docs path",
158168
stacklevel=2,
159169
)
160170
if self.bootstrap_config.swagger_offline_docs:
161-
enable_offline_docs(
162-
self.bootstrap_config.application, static_path=self.bootstrap_config.swagger_static_path
163-
)
171+
enable_offline_docs(application, static_path=self.bootstrap_config.swagger_static_path)
164172

165173

166174
class FastAPIBootstrapper(BaseBootstrapper["fastapi.FastAPI"]):
@@ -189,8 +197,9 @@ async def lifespan_manager(self, _: "fastapi.FastAPI") -> typing.AsyncIterator[d
189197
def __init__(self, bootstrap_config: FastAPIConfig) -> None:
190198
super().__init__(bootstrap_config)
191199

192-
old_lifespan_manager = self.bootstrap_config.application.router.lifespan_context
193-
self.bootstrap_config.application.router.lifespan_context = _merge_lifespan_context(
200+
application = _narrow_app(self.bootstrap_config)
201+
old_lifespan_manager = application.router.lifespan_context
202+
application.router.lifespan_context = _merge_lifespan_context(
194203
old_lifespan_manager,
195204
self.lifespan_manager,
196205
)
@@ -199,4 +208,4 @@ def is_ready(self) -> bool:
199208
return import_checker.is_fastapi_installed
200209

201210
def _prepare_application(self) -> "fastapi.FastAPI":
202-
return self.bootstrap_config.application
211+
return _narrow_app(self.bootstrap_config)

‎lite_bootstrap/bootstrappers/faststream_bootstrapper.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class FastStreamConfig(
7070
faststream_log_level: int = logging.WARNING
7171

7272

73-
@dataclasses.dataclass(kw_only=True, slots=True, frozen=True)
73+
@dataclasses.dataclass(kw_only=True, slots=True)
7474
class FastStreamHealthChecksInstrument(HealthChecksInstrument):
7575
bootstrap_config: FastStreamConfig
7676

@@ -104,7 +104,7 @@ async def _define_health_status(self) -> bool:
104104
return await self.bootstrap_config.application.broker.ping(timeout=5)
105105

106106

107-
@dataclasses.dataclass(kw_only=True, frozen=True)
107+
@dataclasses.dataclass(kw_only=True)
108108
class FastStreamLoggingInstrument(LoggingInstrument):
109109
bootstrap_config: FastStreamConfig
110110

@@ -117,7 +117,7 @@ def bootstrap(self) -> None:
117117
broker.config.logger.params_storage = ManualLoggerStorage(logger)
118118

119119

120-
@dataclasses.dataclass(kw_only=True, frozen=True)
120+
@dataclasses.dataclass(kw_only=True)
121121
class FastStreamOpenTelemetryInstrument(OpenTelemetryInstrument):
122122
bootstrap_config: FastStreamConfig
123123
not_ready_message = OpenTelemetryInstrument.not_ready_message + " or opentelemetry_middleware_cls is empty"
@@ -136,7 +136,7 @@ def _make_collector_registry() -> "prometheus_client.CollectorRegistry":
136136
return prometheus_client.CollectorRegistry()
137137

138138

139-
@dataclasses.dataclass(kw_only=True, frozen=True)
139+
@dataclasses.dataclass(kw_only=True)
140140
class FastStreamPrometheusInstrument(PrometheusInstrument):
141141
bootstrap_config: FastStreamConfig
142142
collector_registry: "prometheus_client.CollectorRegistry" = dataclasses.field(

‎lite_bootstrap/bootstrappers/litestar_bootstrapper.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class LitestarConfig(
116116
swagger_extra_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict)
117117

118118

119-
@dataclasses.dataclass(kw_only=True, slots=True, frozen=True)
119+
@dataclasses.dataclass(kw_only=True, slots=True)
120120
class LitestarCorsInstrument(CorsInstrument):
121121
bootstrap_config: LitestarConfig
122122

@@ -132,7 +132,7 @@ def bootstrap(self) -> None:
132132
)
133133

134134

135-
@dataclasses.dataclass(kw_only=True, slots=True, frozen=True)
135+
@dataclasses.dataclass(kw_only=True, slots=True)
136136
class LitestarHealthChecksInstrument(HealthChecksInstrument):
137137
bootstrap_config: LitestarConfig
138138

@@ -152,7 +152,7 @@ def bootstrap(self) -> None:
152152
self.bootstrap_config.application_config.route_handlers.append(self.build_litestar_health_check_router())
153153

154154

155-
@dataclasses.dataclass(kw_only=True, frozen=True)
155+
@dataclasses.dataclass(kw_only=True)
156156
class LitestarLoggingInstrument(LoggingInstrument):
157157
bootstrap_config: LitestarConfig
158158

@@ -175,7 +175,7 @@ def bootstrap(self) -> None:
175175
self._configure_foreign_loggers()
176176

177177

178-
@dataclasses.dataclass(kw_only=True, frozen=True)
178+
@dataclasses.dataclass(kw_only=True)
179179
class LitestarOpenTelemetryInstrument(OpenTelemetryInstrument):
180180
bootstrap_config: LitestarConfig
181181

@@ -189,7 +189,7 @@ def bootstrap(self) -> None:
189189
)
190190

191191

192-
@dataclasses.dataclass(kw_only=True, frozen=True)
192+
@dataclasses.dataclass(kw_only=True)
193193
class LitestarPrometheusInstrument(PrometheusInstrument):
194194
bootstrap_config: LitestarConfig
195195
missing_dependency_message = "prometheus_client is not installed"
@@ -213,7 +213,7 @@ class LitestarPrometheusController(PrometheusController):
213213
self.bootstrap_config.application_config.middleware.append(litestar_prometheus_config.middleware)
214214

215215

216-
@dataclasses.dataclass(kw_only=True, frozen=True)
216+
@dataclasses.dataclass(kw_only=True)
217217
class LitestarSwaggerInstrument(SwaggerInstrument):
218218
bootstrap_config: LitestarConfig
219219
not_ready_message = "swagger_path is empty or not valid"

0 commit comments

Comments
 (0)