Repository navigation
perf(http): 134 raw aiohttp.ClientSession constructions remain (127 per-request) despite the shared pool at 151 sites #12979
Description
Activity
Step 1 answered: the shared client already supports per-call timeout and headers. No extension needed — step 2 is unblocked.
HTTPClientManager.request()forwards everything through:async def request(self, method, url, *, suppress_error_log=False, **kwargs): ... headers = kwargs.pop("headers", {}) or {} inject(headers) # OTel trace context MERGED into caller headers kwargs["headers"] = headers ... response = await session.request(method, url, **kwargs)
So:
timeout=— passed straight tosession.request(), which accepts a per-requestClientTimeout. A call site doingClientSession(timeout=X)then one request is equivalent toclient.request(..., timeout=X).headers=— supported, and better than the raw form: caller headers are preserved and OTel propagation is merged in, which per-request sessions currently miss entirely.
That last point is worth noting as a second reason to convert beyond connection reuse: every raw
ClientSessioncall site is also missing distributed-trace propagation. The pooling win is the headline; trace continuity is a quiet bonus.Revised assessment of the 134
With timeout and headers covered, the earlier concerns collapse to two genuinely special cases:
category count action plain async with ClientSession()bulk of 127 mechanical conversion custom timeout=/headers=subset of 127 also mechanical — kwargs are supported returns a session for a caller to own ( voice_processing/providers/cloud/base.py:73)1 different lifetime model — review individually assigned / long-lived 7 leave unless a benefit is shown So the conversion is larger in scope but simpler in nature than the issue first suggested: almost all of the 127 are mechanical, not judgement calls.
One caveat to carry into step 2:
request()returns a rawClientResponseand the manager tracks active requests for pool sizing, so converted call sites must not hold responses open beyond their read (the docstring flagsincrement_active()/decrement_active()for streaming). Worth checking any converted site that streams.Step 2 groundwork: there is a connection-lifetime trap, and a safe API that avoids it. Recording before any bulk conversion, because the obvious mechanical substitution is wrong.
The trap
Call sites look like this (
integrations/monitoring_integration.py, 11 occurrences):async with aiohttp.ClientSession() as session: async with session.get(url, headers=..., timeout=...) as response: ...
The inner
async withis what releases the connection back to the pool. The naive conversion —client = get_http_client() response = await client.get(url, headers=..., timeout=...) # raw ClientResponse
— drops that, because
client.get()returns aClientResponserather than yielding one. Converting this way would swap per-request session creation for leaked pooled connections, which is worse: the pool is finite, so leaks exhaust it rather than merely costing a handshake.The safe path
HTTPClientManageralready provides response-consuming helpers that handle release correctly:async def get_json(self, url, **kwargs): async with await self.get(url, **kwargs) as response: response.raise_for_status() return await response.json()
So for the common "fetch JSON" shape, the conversion is a one-liner with no lifetime concern:
data = await get_http_client().get_json(url, headers=..., timeout=...)
Note it also adds
raise_for_status(), which most current call sites do by hand — worth checking each site's existing error handling matches, since this changes a non-2xx from "inspectresponse.status" to "raises".Revised step 2 plan
- JSON-consuming sites →
get_json()/post_json(). Mechanical and lifetime-safe. Likely the bulk of the 59 plain-form sites. - Sites that read text/bytes or inspect
response.status→ must wrap explicitly:async with await client.get(...) as response:. Still mechanical, but theasync withis mandatory and easy to drop. - Streaming sites → additionally need
increment_active()/decrement_active(), per therequest()docstring, or they will fight pool resizing. voice_processing/providers/cloud/base.py:73(returns a session) and the 7 long-lived constructions → individually reviewed, not converted by pattern.
Recommended first batch
integrations/monitoring_integration.py— 11 sites in one file, uniform shape, an integration rather than a hot path, so a mistake is contained and reviewable. Good pilot to establish the pattern before it is applied to the remaining ~48.I have not started the conversion: with the lifetime trap identified, this wants a clear run rather than a partial batch, and getting it wrong trades a performance cost for a stability one.
- JSON-consuming sites →
The pilot file contains both categories — 9 safe, 2 not — which confirms the split-by-response-handling plan is necessary, not cautious.
integrations/monitoring_integration.py: 11ClientSessionsites, 11await response.json(), but only 9raise_for_status().The other two inspect status instead:
:72 if response.status == 200: :83 message=f"Datadog API error: {response.status}", :295 if response.status == 200: :306 message=f"New Relic API error: {response.status}",
So:
shape count conversion raise_for_status()+json()9 await client.get_json(url, headers=…, timeout=…)— semantically exactif response.status == 200:… structured error2 must use async with await client.get(…) as response:Converting all 11 to
get_json()would turn those two from "return a structured error object to the caller" into "raise" — a behaviour change in error paths for Datadog and New Relic integrations, i.e. exactly where a silent change is least welcome.That is 2 sites in 11 (18%) that a uniform pattern would have broken, found in the first file examined. Applied blind across the 127, that shape would have introduced a meaningful number of behavioural regressions in error handling — the kind that only surface when the remote service is already failing.
Conversion recipe, now concrete
# 9 sites — exact data = await get_http_client().get_json(url, headers=self._get_headers(), timeout=ClientTimeout(total=30)) # 2 sites — preserve status inspection client = get_http_client() async with await client.get(url, headers=…, timeout=…) as response: if response.status == 200: ...
Note the second form still needs the
async with— that is the connection-release point, and dropping it is the leak described in my previous comment.Status
Not converted yet. The recipe is settled and the categories are countable per file (
grep -c "raise_for_status()"vsgrep -c "response.status"against theClientSessioncount), so this is now mechanical work with a known trap rather than an exploration. Worth doing in a clear run, per-file, with the counts checked before and after.Pilot file converted — PR #12982. The predicted 9/2 split held exactly; no site fell outside the two categories.
autobot-backend/integrations/monitoring_integration.py: 11 → 0 rawClientSessionconstructions.shape predicted actual conversion raise_for_status()+json()9 9 (7 GET + 2 POST) get_json()/post_json()if response.status == 200:→ structured error2 2 (both test_connection())async with await client.get(...)The POST sub-split was not visible from the
raise_for_status()grep — 2 of the 9 arepost_json()(_create_monitor, and the New Relic NRQL GraphQL query). Worth folding into the per-file counting recipe:grep -c "session.post("alongside the existing two counts.The lifetime trap is real and now has a check for it. After 12 requests through the shared pool the local exercise reports:
pooled idle conns=1 still-acquired=0One connection reused across all 12, zero still acquired.
still-acquiredis a cheap, direct assertion for the leak described above — readingsession.connector._acquired_per_hostafter a converted call path is a better gate than eyeballing the diff for a missingasync with, and is worth running per-file for the remaining ~48.Behaviour-change check confirmed necessary. The 503 branch was exercised directly:
dd.test_connection 503 -> IntegrationStatus.ERROR | Datadog API error: 503 | {'error': 'nope'}A uniform
get_json()sweep would have raised there instead. 2-in-11 in the first file, as predicted.One thing the pilot surfaced that changes the cost/benefit
Filed #12981:
HTTPClientManager._active_requestsincrements on everyrequest()but decrements only on the error path, andget_json/post_jsonnever decrement — the stats after 12 clean requests readactive_requests: 12. Exactly one caller in the repo (chat_workflow/manager.py) honours the contract, against ~151 call sites.Relevant to this issue rather than merely adjacent: every conversion done here adds another permanent increment to a counter that drives pool auto-sizing (
utilization = _active_requests / _current_pool_size) and gates deferred pool recreation. Converting the remaining ~48 sites makes the shared pool ratchet toward_pool_maxfaster than it does today. The pooling win is still real — connections are genuinely released — but #12981 is worth fixing before or alongside the bulk conversion, not after it, or the sweep degrades the metric it is meant to improve.Pre-existing, so not fixed in #12982 — a one-file pilot is the wrong place to change behaviour for all 151 sites.
- added a commit that references this issue
on Jul 29, 2026 Pilot converted — PR #12982 merged as
8e2203c2a.integrations/monitoring_integration.py: 11ClientSession(→ 0. Issue stays open for the remaining ~116 sites.The predicted split held exactly, with one addition to the recipe
shape count conversion raise_for_status()+json(), GET7 get_json()raise_for_status()+json(), POST2 post_json()if response.status == 200:→ structured error2 async with await client.get(...)Recipe amendment: 2 of the "9 safe" sites are POSTs, which the
grep -c "raise_for_status()"triage does not distinguish. Addgrep -c "session.post("to the per-file count.A better way to verify the leak trap
Rather than eyeballing diffs for a missing
async with, assert it directly. After exercising all 11 converted paths against a local aiohttp server:pooled idle conns=1 still-acquired=0 (12 requests)Reading
connector._acquired_per_hostis a cheap, direct assertion that connections release. Recommend reusing this on every subsequent batch — it catches the exact failure mode that would make this work counter-productive.The 2-site carve-out was necessary, not cautious
Confirmed by exercise: the 503 branch still returns
IntegrationStatus.ERROR | Datadog API error: 503. A uniformget_json()sweep would have raised there instead — a silent behaviour change in the error path of an integration, i.e. exactly where it is least visible.Sequencing changed: #12981 must land first
The pilot surfaced #12981 —
_active_requestsincrements on every request but decrements only on the error path;get_json/post_jsonnever do. Verified independently: repo-wide, exactly one production caller invokesdecrement_active()against ~151 sites.That counter is not cosmetic. It drives
utilization = _active_requests / _current_pool_size(pool auto-sizing) and gatesif self._active_requests == 0for deferred session recreation — which, with a monotonically rising counter, can never fire.So every conversion currently adds a permanent increment. Converting the remaining ~116 sites before fixing #12981 would push the pool toward
_pool_maxand permanently defer recreation — degrading the metric this work exists to improve. PR #12989 fixes it and should land before the next batch.Batch 2 converted — PR #12991.
autobot-backend/utils/:connection_utils.py5 -> 0,hardware_metrics.py3 -> 0. Issue stays open.The split was 8/8 the "hard" shape — the opposite of the pilot
shape pilot (11 sites) this batch (8 sites) raise_for_status()+json(), GET7 0 raise_for_status()+json(), POST2 0 inspects response.status, returns structured result2 8 (7 GET + 1 POST) Not one site here matched the
get_json()/post_json()shape. The pilot's 82%-safe/18%-careful ratio is not a property of the codebase — it is a property of integration clients, which are written against documented APIs and so raise. Health-probe code is the inverse: its whole job is to turn an unreachable service into a status string, so it inspects rather than raises. Expect the per-shape split to swing hard by the kind of module, and count per file rather than extrapolating from the pilot.Recipe amendments
1.
tracked_request()supersedesasync with await client.get(...)for the status-inspecting shape. The recipe in my earlier comment predates #12989.client.get()underasync withreleases the connection but still leaves_active_requestspermanently incremented — the same defect #12981 described and #12989 fixed for the*_json()helpers.tracked_request()owns the response block and balances the counter:async with get_http_client().tracked_request("GET", url, timeout=t) as response: if response.status == 200: ...
Measured difference after 16 requests: counter reads 0 with
tracked_request(), would read 16 with the literal recipe. Every remaining batch should use it for shape 3.2. The triage grep mis-classifies one shape.
_test_ollama_model()is a POST with noraise_for_status()that readsjson()on 200 andtext()on non-200.grep -c "session.post("flags it as a POST and the absentraise_for_status()suggests shape 3 — but a reviewer skimming for "POST + json()" could easily reach forpost_json(), which would raise instead of returning the"partial"result. Check what the non-2xx branch reads, not just whetherraise_for_status()is present.3. Log level is part of the conversion, not incidental.
request()logs failures at ERROR by default. Five of these eight sites deliberately swallow failure — into"disconnected",{}, a999.0latency sentinel, or a rate-limited warning that exists specifically to stop Ollama-not-running spam. Converting them withoutsuppress_error_log=Trueturns a quiet expected condition into per-probe ERROR noise on a 5-second poll loop. Worth adding to the per-file checklist: for each site, does the existing code swallow, downgrade, or rate-limit the failure? If so, passsuppress_error_log=True.Leak assertion (the pilot's method, reused)
All 8 sites exercised against a local aiohttp server on both success and failure branches:
requests issued : 16 pooled idle conns : 1 still-acquired : 0 active_requests counter: 016 requests over one connection, zero still acquired, counter balanced. The error branches returned
'disconnected','disconnected',False,[],'partial'— all structured, no raises, confirming the 8/8 carve-out was necessary.Existing tests: 14 passed before, 14 passed after (
utils/connection_utils_test.py).A pre-existing bug that made one converted site dead — #12990
hardware_metrics.pyread threeVMConfigattributes that do not exist (vm.npu_worker,vm.ai_stack; the fields arenpu,aistack). Two silent failures:_get_npu_worker_stats()was permanently dead — theAttributeErrorwas swallowed byexcept Exception: return {}, so the NPU worker was never contacted._get_service_configs()raised, killingcollect_service_performance_metrics()for all six services.
Fixed in #12991 because one of the three lines was a line being converted, and the conversion there was otherwise unverifiable — the request never fired. This is a second argument for the exercise-it-locally verification step: a purely diff-based review would have passed a conversion of a code path that cannot execute.
Remaining, with the scope now pinned
The headline reconciles exactly against
autobot-backend/non-test: 134 - 11 (pilot) - 8 (this batch) = 115, confirmed bygrep -rh "aiohttp.ClientSession(" --include=*.py autobot-backend \ --exclude="*_test.py" --exclude="test_*.py" | wc -l -> 115But that scope is narrower than the issue's title implies. Repo-wide non-test is 166 — roughly 51 further constructions in
autobot-infrastructure/(18 files),autobot-slm-backend/(8),plugins/(3),autobot-npu-worker/(2) andautobot_shared/(2) that the headline never counted. Worth deciding now whether they are in scope, rather than finding the gap whenautobot-backend/hits zero and the issue looks finished.Densest clusters left:
integrations/cicd_integration.py 14 integrations/version_control_integration.py 11 services/telegram_bot_service.py 8 services/tts_client.py 7 integrations/communication_integration.py 5 web_fetch/ (fetcher, robots, site_mapper) 4cicd_integration.pyandversion_control_integration.pyare the natural next batch — same integration-client character as the pilot, so expect the split to swing back towardget_json().Batch 3 converted — PR #12994.
integrations/cicd_integration.py14 → 0,integrations/version_control_integration.py11 → 0. Issue stays open.The split swung back toward
get_json(), as predicted for integration clientsshape pilot (11) utils (8) this batch (25) raise_for_status()+json(), GET7 0 16 raise_for_status()+json(), POST2 0 2 inspects response.status→ structured result2 8 5 raise_for_status()+text()0 0 1 POST + raise_for_status(), body never read0 0 1 18 safe / 7 careful. The "count per file, do not extrapolate" rule holds across three batches now: 82% safe → 0% safe → 72% safe, tracking the kind of module rather than anything about the codebase.
The triage amendment caught one — in the opposite direction to the utils batch
The utils batch warned that an absent
raise_for_status()does not imply shape 3. This batch found the mirror image: the presence ofraise_for_status()does not imply a*_json()candidate.JenkinsIntegration._trigger_build()is a POST withraise_for_status()that returns{"status": "triggered", ...}and never reads the body. Jenkins answersbuildWithParameterswith an empty201. Exercised against a server returning exactly that:post_json RAISED ContentTypeError: 201, message='Attempt to decode JSON with unexpected mimetype: text/plain; charset=utf-8' tracked_request -> status 201 | site returns {'status': 'triggered', ...}A sweep classifying on
session.post(+raise_for_status()would have converted this topost_json()and broken the success path — not the error path, which is where both previous batches' traps lived. The general form of the rule:Classify on what the site reads, in both branches.
raise_for_status()says only "non-2xx raises"; it says nothing about whether the 2xx body is JSON, text, or absent._get_build_log()is the milder version of the same thing —raise_for_status()present, but it readstext()because Jenkins console output is plain text.Leak assertion (third batch, same method)
All 25 sites exercised on both branches against a local aiohttp server, 52 requests:
requests issued : 52 pooled idle conns : 1 still-acquired : 0 active_requests counter: 0tracked_request()used throughout for the careful shape. All fivetest_connection()sites still return structuredIntegrationHealthon 503 rather than raising; all 18*_json()sites raiseClientResponseError, exactly asraise_for_status()did before.Existing tests: 186 passed before, 186 after (baseline taken by
cp-swapping the origin files in).Two discoveries filed
- perf(http): monitoring_integration test_connection sites leak _active_requests (pre-#12989 recipe) #12992 — the pilot's own two sites (
monitoring_integration.py:69,:278) still use the pre-fix(http): balance _active_requests in response-consuming helpers (#12981) #12989async with await get_http_client().get(...)form. Connection released, counter leaked: 10 requests →active_requests: 10. Merged code, currently accruing on every Datadog/New Relic health poll. Worth folding into the next batch rather than leaving the pilot as the one converted file that still exhibits the defect this work exists to remove. - test: integration_unknown_action_contract_test discovery is CWD-relative, fails when run from autobot-backend/ #12993 —
integration_unknown_action_contract_test.pydiscovers subclasses via a CWD-relative path, so the suite fails fromautobot-backend/and passes from the repo root. Cost a baseline cycle to rule out; pre-existing onorigin/Dev_new_gui.
Remaining
134 - 11 (pilot) - 8 (utils) - 25 (this batch) = 90 in autobot-backend/ non-testDensest clusters left:
services/telegram_bot_service.py 8 services/tts_client.py 7 integrations/communication_integration.py 5 web_fetch/ (fetcher, robots, site_mapper) 4communication_integration.pyis the natural next batch if the goal is to finishintegrations/— same module kind, so expect another safe-heavy split, and it pairs naturally with the #12992 fix in the same directory.telegram_bot_service.pyandtts_client.pyare the larger prize but a different module kind again; count per file.The scope question from the previous comment is still open: repo-wide non-test is ~51 further constructions outside
autobot-backend/that the 134 headline never counted. Worth deciding beforeautobot-backend/reaches zero and the issue looks finished.- perf(http): monitoring_integration test_connection sites leak _active_requests (pre-#12989 recipe) #12992 — the pilot's own two sites (
Correction to the "Remaining" section of my previous comment — and the subtraction method behind it is unsound.
I wrote
134 - 11 - 8 - 25 = 90. Measured against the actual tip, it is 98:$ git grep -h "aiohttp.ClientSession(" origin/Dev_new_gui -- 'autobot-backend/**/*.py' ':!*_test.py' ':!*/test_*.py' | wc -l 123 # origin tip, before this batch $ grep -rh "aiohttp.ClientSession(" --include=*.py autobot-backend --exclude="*_test.py" --exclude="test_*.py" | wc -l 98 # this branch, 25 removed123 - 25 = 98. The arithmetic was right; the baseline was stale.
Why the subtraction drifts
The previous batch reconciled its count exactly (
134 - 11 - 8 = 115) and reported the grep confirming it. That reconciliation was true when it was taken and is false now: the origin tip has since gained ~8 further constructions from unrelated PRs landing inautobot-backend/.So this is not a one-off arithmetic slip — the headline 134 is a snapshot, not a budget. New
aiohttp.ClientSession(sites are being added by other work at a rate comparable to a small batch. Subtracting conversions from a fixed starting number will keep reporting progress that is better than reality, and the error compounds silently across batches.Two consequences worth acting on:
-
Re-measure per batch; never subtract. Every future batch comment should quote the grep taken against its own base, not
previous_total - converted. I have corrected this batch's figure accordingly. -
The tail will not close by conversion alone. If new per-request sessions keep arriving, reaching zero requires a gate as well as a sweep — a lint rule or CI check rejecting new
aiohttp.ClientSession(outsideautobot_shared/http_client.py, with the handful of legitimate long-lived constructions explicitly allow-listed. Without it, this issue can be worked indefinitely without converging. Worth deciding whether that gate is in scope here or belongs in its own issue before the remaining ~98 are picked off.
Everything else in the previous comment — the 18/7 split, the
_trigger_buildfinding, and the leak/counter assertions (52 requests, still-acquired 0, counter 0) — was measured directly on this branch and stands unchanged.Also filed while shipping this batch: #12995 — the
Check PR links to its issuegate accepts onlyResolves/Closes/Fixes, so a batch PR cannot pass it without wrongly closing this issue. Both #12991 and #12994 ship with that check red for exactly this reason.-
23 remaining items
- added a commit that references this issue
on Jul 30, 2026 Count correction — the title figure is stale.
Measured against
origin/Dev_new_gui(65760c316, 2026-07-31), excluding tests and conftest:git grep -n 'aiohttp.ClientSession(' -- '*.py' | grep -vE '_test\.py|/tests?/|conftest' | wc -l 7373 raw constructions remain, not the 134 in the title.
Nine batches have merged under this issue: #12982, #12991, #12994, #12999, #13000, #13001, #13002, #13003, #13006 — plus #13046, which fixed the ceiling regression tracked as #13041.
Issue stays open: 73 sites is real remaining work. Flagging only so the title is not read as the current baseline, and so the next batch starts from the right number.
Found during a worktree/issue reconciliation sweep — all nine batch branches were verified fully integrated into
Dev_new_guiand their local worktrees removed.Progress update — both figures have dropped substantially since filing. Re-measured with the same two counts this issue tracks:
metric at filing now change raw aiohttp.ClientSession(...)constructions134 84 −50 of which per-request ( async with)127 62 −65 grep -rn 'aiohttp.ClientSession(' --include=*.py autobot-backend/ autobot_shared/ autobot-slm-backend/ -> 84 grep -rn 'async with aiohttp.ClientSession(' ... -> 62So roughly half the per-request cases the issue identified as "the bulk" have already been converted, presumably alongside other work rather than under this issue. Not closing — 84 remain and the shared-client migration is not finished — but the remaining scope is materially smaller than the body describes, which matters for whoever picks it up.
The 134/127 in this issue is stale. Measured figure, with the selector named.
AST walk (
ast.Callwhose func is*.ClientSessionor a bareClientSession) over tracked*.pyonorigin/mainat0a4cffced:scope files constructions whole tree 61 99 non-test 36 59 non-test, inside autobot-backend/11 12 non-test, outside autobot-backend/25 47 git grep -c 'aiohttp\.ClientSession('reads 70 files / 115 occurrences over the same tree. The 16-occurrence gap is prose — docstrings, a log string and a regex replacement template — which is the reason the counts here are parsed rather than matched.Two things the breakdown makes visible that the single number hid:
- The backend tail is drained. All 12 remaining
autobot-backend/sites are documented carve-outs (SSRF-pinnedconnector=, custom TLS context, or a session held across calls), andtests/test_raw_client_session_ceiling_12992.pyratchets them at 13. - The other 47 — four fifths of the population — were under no ratchet at all. That guard only ever read
autobot-backend/.
PR #17973 (draft) converts 9 of the 47 in the two core plugins and puts the remaining 35 under an inventory ratchet covering everything outside
autobot-backend/. It usesRefs, notCloses.Not established: whether the 23 sites in
autobot-infrastructure/shared/scripts/are deliberate. They have the structural shape of operator entrypoints, which is all the inventory asserts about them; most are probably unconverted work rather than carve-outs, and nothing in this pass proved either way.- The backend tail is drained. All 12 remaining
- added 4 commits that reference this issue
on Oct 10, 2026 Progress: #17973 merged. Nine plugin call sites (image generation 3, video generation 6) now use the pooled client through
tracked_request, each with an explicit per-call timeout. The timeouts come fromautobot_shared/generation_http_timeouts.py, env-backed and validated as finite and positive: 300 s for the synchronous Stability generation, 30 s for submit and poll, 10 s for connect. The raw-session inventory ratchet (repo_tests/raw_aiohttp_session_inventory_12979_test.py) pins the remaining 35 raw sites in 21 files.plugins/core-pluginsis now collected in CI. The acceptance criteria stay open: per the PR, the remaining raw constructions are not yet converted or individually justified.
Split out of #10603 after auditing its headline table. This is the only row with substantial, unblocked work remaining — the other rows are either already delivered or gated on decisions elsewhere (see the audit comments on #10603).
Problem
autobot_shared's pooled HTTP client is widely adopted, but a large tail still constructs a session per request:Each
async with aiohttp.ClientSession()builds a fresh connector, resolves DNS, and re-establishes TLS for what is often a single request — the cost the shared pool exists to avoid.Where they are
Spread rather than clustered, which is why this has not been picked off incidentally:
Also present in
web_fetch/(fetcher, robots, site_mapper),voice_processing/(several providers), andutils/hardware_metrics.py.Why this needs care rather than a sweep
Not every raw construction is wrong, and a blind conversion would change behaviour:
ClientSession(timeout=...)) — the shared client must support an equivalent per-call override or those calls change their failure characteristics;web_fetch/fetcher.py:174);voice_processing/providers/cloud/base.py:73returns a session for a caller to own, which is a different lifetime model from a shared pool;async withsites are long-lived by design and may be correct as-is.So the work is: confirm the shared client covers timeout/header overrides, convert the plain per-request cases first (the bulk of the 127), and treat the custom-configuration and returned-session cases individually.
Suggested sequencing
get_http_client()supports per-call timeout and headers; extend it if not.async with aiohttp.ClientSession()cases with no custom config — mechanical, low risk.Done when
Refs #10603.