perf(core): build the boto3 clients on first use, not at import - #31
Merged
Conversation
A fifth sweep read the suite's own `--durations` output. Every subprocess test paid a flat
~4.5s that was not work:
import sentinel_harness 4.50s at 7% CPU (not computing — waiting)
python -c "pass" 0.03s
`-X importtime` put 4.15s of self time in `sentinel_harness.core`: two module-level
`boto3.client(...)` calls.
MY FIRST ATTRIBUTION WAS WRONG, and the control test I wrote is what caught it. I assumed
service-model parsing (153 operations). That test asserted "eager construction costs > 2s"
and FAILED, reporting 0.09s — because inside pytest the credential chain is already
satisfied by fake keys and short-circuits. Measured properly, in matched environments:
no credentials 4.46s the full chain, ending at IMDS
fake credentials 0.31s the first provider hits
AWS_EC2_METADATA_DISABLED=true 0.32s
The cost is the **instance-metadata probe timing out**. So importing this library made it
reach for 169.254.169.254 — the exact address its own INV-EGRESS family exists to refuse.
That reframes the finding: it is a correctness and least-surprise defect that happened to
show up as slowness.
Three consequences, the third mattering most:
1. 26 subprocess tests x 4.5s ~ 115s of a 202s suite, building clients nothing used.
2. Importing a library read ~/.aws/ and probed the network. A library must not.
3. It shaped the tests: real clients existed before any test ran, so the only way to
substitute a fake was to patch the module global afterwards — which ~48 files do.
`_LazyClient` defers construction to first attribute access. Deliberately a proxy, not a
function, so nothing about the call sites or the ~48 patching tests changes.
import sentinel_harness 4.50s -> 0.26s (17x)
full suite 202s -> 107s (1.9x)
scenario + CLI E2E layer 107s -> 8.9s (12x)
`set_region` also stays lazy: the CLI's --region flag runs before any AWS work, including
for the offline detection commands that never touch AWS, so building there would put the
cost back on every command's startup.
Two of my own mistakes, both recorded in the code:
- The first proxy used `__slots__` and broke
`test_gateway.py::test_scenario_named_supervisor_imports_without_aws`, which patches a
method ON the client rather than replacing it. That is a legitimate pattern — it proves
a scenario import makes no AWS call — so the proxy must support it. `__slots__` removed.
- `Any` was used in annotations without importing it. It ran only because
`from __future__ import annotations` makes annotations strings; mypy gates core.py, so
it is now imported properly.
tests/test_lazy_clients.py pins both properties: import is under budget, no client is built
at import, and boto3.client is FORBIDDEN during import of the package + gateway +
registry_live + mcp_server. Plus six transparency tests covering every pattern the ~48
existing files rely on. Mutation-tested 4/4: reverting to eager construction, making
set_region eager, breaking the cache, and hardcoding the region are each caught.
Tested: 3725 passed / 6 skipped in BOTH fixed and random order; installed-wheel E2E 8/8;
scenario E2E 25/25; README CLI E2E 13/13; IaC E2E tsc 0 / 8-of-8 / synth 9 stacks; ruff
clean; both mypy gates clean; make ci green.
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.
A fifth sweep read the suite's own
--durationsoutput. Every subprocess test was paying a flat ~4.5s that was not work:-X importtimeput 4.15s of self time insentinel_harness.core: two module-levelboto3.client(...)calls.My first attribution was wrong, and my own control test caught it
I assumed service-model parsing (153 operations). The control asserted "eager construction costs > 2s" and failed, reporting 0.09s — because inside pytest the credential chain is already satisfied by fake keys and short-circuits. Measured properly, in matched environments:
AWS_EC2_METADATA_DISABLED=trueThe cost is the instance-metadata probe timing out. So importing this library made it reach for
169.254.169.254— the exact address its own INV-EGRESS family exists to refuse.That reframes the finding: it is a correctness and least-surprise defect that happened to show up as slowness.
Three consequences, the third mattering most
~/.aws/and probed the network. A library must not.The fix
_LazyClientdefers construction to first attribute access. Deliberately a proxy, not a function, so nothing about the call sites or the ~48 patching tests changes.import sentinel_harnessset_regionalso stays lazy: the CLI's--regionflag runs before any AWS work — including for the offline detection commands that never touch AWS — so building there would put the cost back on every command's startup path.Two of my own mistakes, both recorded in the code
__slots__and broketest_gateway.py::test_scenario_named_supervisor_imports_without_aws, which patches a method on the client rather than replacing it. That is a legitimate pattern — it proves a scenario import makes no AWS call — so the proxy must support it.__slots__removed, with the reason.Anywas used in annotations without importing it. It ran only becausefrom __future__ import annotationsmakes annotations strings; mypy gatescore.py, so it is now imported properly.Testing
tests/test_lazy_clients.pypins both properties: import is under budget, no client is built at import, andboto3.clientis forbidden during import of the package +gateway+registry_live+mcp_server. Plus six transparency tests covering every pattern the ~48 existing files rely on.Mutation-tested 4/4 — reverting to eager construction, making
set_regioneager, breaking the cache, and hardcoding the region are each caught.tscexit 0 · 8/8 stack tests ·cdk synth9 stacksruffclean · both mypy gates clean ·make cigreen