feat(comicvine): release its connections and sqlite handles on close

[ajslater]

Answers F12: a Comic Vine counterpart to OnlineSession.close(). v5.2.0's
release call covered Metron only.

What a Comic Vine client holds

simyan's Comicvine._session is a CachedLimiterSession(CacheMixin, LimiterMixin, Session) — requests-backed, like mokkari's. Per credential
set, after a run that touches every endpoint comicbox calls, it holds:

• the urllib3 connection pool
• 1 requests_cache sqlite connection (the response cache)
• 5 pyrate_limiter SQLiteBucket connections — one per endpoint pool
  (issues, volumes, get_issue, get_volume, search), the same
  bucket_% tables _read_rate_limit_buckets already reads
• pyrate-limiter's daemon leaker thread

Comicvine exposes no close of its own, so this goes through _session
— private simyan surface, as _maintain_cache already does.

Why the one-liner would have been worse than nothing

session.close() does cascade, and gets the response cache, the sockets and
the leaker thread. It does not get the buckets: requests-ratelimiter asks
pyrate-limiter to close them, but BucketFactory.close() drops its leaker
before iterating get_buckets(), which reads that leaker — so the loop sees
an empty list (pyrate-limiter 4.5.0).

What it does do is drop the references that had been keeping the buckets
alive. Measured ResourceWarning: unclosed database at interpreter exit,
same workload:

close	warnings
none (before this PR)	0
session.close() only	5
this PR	0

A session-only close would have made the reported symptom louder, not
quieter.

What this does

close_shared_sessions() in the Comic Vine source, mirroring metron_api's,
wired into OnlineSession.close() (and its context manager) and the CLI's
end-of-run release. Per client: close the session, empty the factory's bucket
registry, then close each bucket. Emptying first is what keeps a closed
bucket from being handed back to a racing lookup, which would die on its
None connection; each bucket's own lock covers an acquire already in
flight.

A closed client stays usable, as Metron's does — the response cache reopens
lazily and the hourly budget lives in the bucket file, so a rebuilt bucket
resumes where the closed one left off. The sharp edge, documented on
OnlineSession.close(): a Comic Vine lookup that already holds a bucket can
fail outright rather than merely reconnect, so close between files, not under
one. Codex's _release_metron_connections calls this at session teardown,
which fits.

Tests

tests/unit/test_comicvine_session_lifecycle.py, mirroring the Metron one:
doubles for the ordering and tolerance contracts, and real-client tests that
every sqlite handle is released, that a closed client still works and still
reports its spent budget, and that the close emits no ResourceWarning.
Verified they fail without the bucket close (4 of 11, including the warning
one, which reproduces unclosed database in <sqlite3.Connection ...>).

tests/util's close_comicvine_client had grown the same dance for test
hygiene; it now delegates to the production close.

make fix, make lint, make ty clean; 2252 passed, 1 skipped.

Adjacent, not fixed here

mokkari's close() is sockets-only by design and explicitly leaves a
caller-supplied cache alone, and mokkari.sqlite_cache.SqliteCache has no
close() at all while holding its connection open for the process. comicbox
builds that cache itself, so one unclosed sqlite connection remains on the
Metron side — same warning class, one handle instead of five.

🤖 Generated with Claude Code