refactor: mechanical readability pass (#199) - #206
Merged
Conversation
- alias `config = self.bootstrap_config` in every method that uses it more than twice, across the bootstrappers and the OpenTelemetry, Sentry, Pyroscope and logging instruments - replace the three transcriptions of the teardown-error idiom with one `collect_teardown_errors()` context manager next to `TeardownError` - move `BaseBootstrapper.__init__` under the class attributes and extract its instrument-selection loop into `_select_instruments()` - delete the unreferenced `types.BootstrapObjectT`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #199.
Four independent cleanups from the same read-through, no behaviour change in any of them.
1.
self.bootstrap_config.as line noiseA
config = self.bootstrap_configlocal in every method that uses the prefix more thantwice. The issue named four files; this went repo-wide once the same shape turned up in
fastmcp_bootstrapper.pyand in the Sentry, Pyroscope and logging instruments — theSentry
bootstrap()alone spent the prefix 12 times. An AST sweep now reports no methodover the threshold. Methods at exactly two uses are left alone, which is where the issue
drew the line.
Plain
config =, neverconfig: typing.Final =:CorsInstrument.bootstrap()alreadyspelled it that way before this change, so the codebase had a convention to match.
2. The teardown-error idiom is transcribed three times
One
collect_teardown_errors()context manager inexceptions.py, next to the error itraises. It yields a collector whose
capture(name)records one step's failure and letsteardown continue; the optional
loggerargument carriesbase.py's per-failure warning,which the two instruments deliberately do not emit.
Both nested
finallyshapes inLoggingInstrument.teardown()are preserved verbatimrather than folded into
capture().capture()swallowsException, so thefinallysare what still guarantee
_detach_record_filters()and the factory reset run when aBaseException— aKeyboardInterruptmid-teardown — passes through.raise ... from collector.errors[0][1]still fires outside anyexcept, so__cause__is the first failure and
__context__staysNone, as in all three originals.One observable drift worth naming: the teardown warning's
LogRecord.pathnameandfuncNamenow point atexceptions.py. Same logger name, same message; nothing assertsit.
3.
BaseBootstrapperreads out of order__init__moved directly under the class attributes, and its instrument-selection loopextracted to
_select_instruments().That extraction adds a stack frame, so the
InstrumentDependencyMissingWarningstacklevelhad to go 3 -> 4 to keep landing on the line that constructed thebootstrapper. The literal now depends on every bootstrapper defining its own
__init__,including
FreeBootstrapper's puresuper()pass-through — delete that one-linedelegation and the warning silently re-points at lite-bootstrap's own source. Pinned as an
invariant in
test_free_bootstrap.py, which fails if that__init__goes away.4.
types.BootstrapObjectTis deadDeleted. No references in the package, tests, benchmarks or docs.
ApplicationTin thesame file is live and stays.
Tests
tests/test_exceptions.pyis new: the collector's contract is not fully reachable throughthe three call sites, and the repo gates on 100% coverage. It covers aggregation and
chaining, the silent path, the logging argument, and an invariant that
capture()letsevery non-
ExceptionBaseExceptionthrough — broadening thatexceptwould maketeardown uninterruptible.
ruff format,ruff check,ty checkclean; 275 tests pass at 100% statement coverage.Branch coverage is unchanged from
mainat 99.10%, the same 32 partial branches, all ofthem module-level
if import_checker.is_X_installed:guards.