Skip to content

Feature/types and split#27

Closed
Pavkazzz wants to merge 23 commits intomasterfrom
feature/types-and-split
Closed

Feature/types and split#27
Pavkazzz wants to merge 23 commits intomasterfrom
feature/types-and-split

Conversation

@Pavkazzz
Copy link
Copy Markdown
Collaborator

@Pavkazzz Pavkazzz commented Mar 13, 2026 •
edited
Loading

Problem

The hasql/base.py module (~750 lines) contained everything: the abstract BasePoolManager class, protocols (AcquireContext), connection contexts (TimeoutAcquireContext, PoolAcquireContext), constants, and pool health monitoring logic. Each driver (aiopg, asyncpg, psycopg3, asyncsqlalchemy) was implemented by subclassing BasePoolManager and overriding ~10 abstract methods, which led to:

  • Tight coupling: adding a new driver required inheriting the entire BasePoolManager
  • Poor testability: impossible to test a driver in isolation from the pool manager
  • Weak typing: pervasive use of Any and # type: ignore instead of generics
  • Flat structure: all drivers lived in the package root (hasql/aiopg.py, hasql/asyncpg.py, etc.)

Solution

Architecture: composition over inheritance.

  1. Split base.py into focused modules:

    • hasql/abc.py — abstract PoolDriver[PoolT, ConnT] interface with generics
    • hasql/acquire.py — AcquireContext, TimeoutAcquireContext, PoolAcquireContext
    • hasql/constants.py — DEFAULT_REFRESH_DELAY, DEFAULT_ACQUIRE_TIMEOUT, etc.
    • hasql/health.py — PoolHealthMonitor (master/replica role monitoring)
    • hasql/pool_manager.py — concrete BasePoolManager that accepts a driver: PoolDriver

  2. Extract drivers into hasql/driver/ package:

    • hasql/driver/aiopg.py, hasql/driver/asyncpg.py, hasql/driver/psycopg3.py, hasql/driver/asyncsqlalchemy.py, hasql/driver/aiopg_sa.py, hasql/driver/asyncpgsa.py
    • Each driver implements the PoolDriver ABC
    • PoolManager is a thin wrapper that passes the appropriate driver to BasePoolManager

  3. Type safety:

    • Removed all type: ignore and Any in favor of Generic[PoolT, ConnT]
    • Balancer policies are fully generic-typed

  4. Backward compatibility:

    • No backward at all

Pavkazzz and others added 4 commits March 11, 2026 12:19
@Pavkazzz @claude
refactor(types): remove type: ignore and Any usages, add generics
a61b422 
- Add AcquireContext Protocol and make TimeoutAcquireContext generic
- Make BasePoolManager, PoolAcquireContext, AbstractBalancerPolicy generic
  over PoolT/ConnT with proper type annotations on all abstract methods
- Make Stopwatch generic over key type, fix _cache type (int -> float)
- Make balancer policies generic over PoolT
- Fix asyncsqlalchemy release_to_pool param name to match base signature
- Fix psycopg3 _is_master with explicit None check for fetchone()
- Narrow asyncpg import ignore to type: ignore[import-untyped]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pavkazzz @claude
refactor(core): split monolithic base.py into focused modules and ext…
6abd17c 
…ract driver package

Extract BasePoolManager internals into dedicated modules (abc, acquire,
constants, health, pool_manager) and move driver implementations from
hasql/*.py into hasql/driver/ package. Introduce PoolDriver ABC with
generics to replace abstract methods on pool manager. Bump to v0.10.0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pavkazzz @claude
feat(metrics): add enriched pool metrics model, OTLP export, and docs
5c05fa4 
- Add PoolStats, PoolMetrics, HasqlGauges dataclasses to metrics.py
- Add pool_stats() to PoolDriver ABC, deprecate driver_metrics()
- Rewrite BasePoolManager.metrics() with per-pool enrichment
  (role, health, response_time, in_flight, driver-specific extras)
- Track context-managed connections for accurate in_flight counts
- Extract PoolState into dedicated pool_state.py module
- Break balancer_policy → pool_manager import cycle via
  PoolStateProvider protocol
- Add OTLP example scripts for all 5 drivers (example/otlp/)
- Add metrics + OTLP sections to README.rst
- Add docs/metrics-dashboard-example.md with Grafana layout guide
- Merge migration guides into docs/migration-0.9.0-to-0.11.0.md
- Maintain backward compat: Metrics.drivers property, driver_metrics()

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pavkazzz @claude
fix(core): address code review findings across pool management and dr…
5a3604b 
…ivers

Fix 11 bugs found during code review:
- acquire.py: use covariant TypeVar in AcquireContext protocol, guard __aexit__ against None state
- pool_state.py: fix TOCTOU race in wait_for_master/replica_pools using wait_for inside lock
- metrics.py: use defaultdict(float) for _acquire_time, wrap yields in try/finally
- utils.py: preserve scheme in Dsn.with_() instead of dropping to default
- driver/asyncpg.py: use .get() fallback to avoid KeyError on cache miss
- driver/psycopg3.py: guard putconn against None connection
- driver/asyncsqlalchemy.py: handle None host, support postgres:// scheme alias
- pool_manager.py: fix _clear() to snapshot connections before clearing dict, fix error message
- docs: rename migration guide to 0.10.0, document broken driver import paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mosquito mosquito requested review from hugiron and mosquito March 13, 2026 15:45
Pavkazzz and others added 19 commits March 16, 2026 18:55
@Pavkazzz @claude
refactor(api): reduce BasePoolManager public API surface
edb9f91 
Reduce public API from 18+ methods/properties to 8 public methods
plus __aenter__/__aexit__. Add proxy methods (ready, wait_masters_ready,
available_pool_count) on the manager for convenience. Remove deprecated
public accessors and proxy methods, privatize internal attributes.

- Rename pool_state → _pool_state, balancer → _balancer, etc.
- Add ready(), wait_masters_ready(), available_pool_count proxy
- Remove release(), terminate(), __iter__, driver, host(), etc.
- Privatize register/unregister_connection
- Update all internal callers (health.py, acquire.py)
- Update tests, migration docs, and README

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pavkazzz
pool state + class-scheme
4298df7
@Pavkazzz @claude
refactor(core): modernize types, fix asyncpg cache bug, harden acquir…
1847629 
…e cleanup

- Replace asyncpg cached_hosts dict[int, str] with WeakKeyDictionary
  to prevent stale lookups on id(pool) reuse after GC
- Modernize type annotations: Optional[X] → X | None, Dict → dict,
  List → list, Set → set, imports from collections.abc
- Rename balancer _pool_manager → _pool_state to match actual type
- Return new dict from prepare_pool_factory_kwargs (no input mutation)
- Defensive copies of dsn_list and pool_factory_kwargs in PoolState
- Clean up driver context on _register_connection failure in acquire
- Add PoolManagerClosingError instead of misusing CancelledError
- Simplify driver tests to test drivers directly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pavkazzz @claude
refactor(core): address code review findings
30fca2f 
- Inject dependencies into PoolAcquireContext instead of
  accessing pool_manager private fields (encapsulation)
- Make all PoolAcquireContext fields private
- Remove return from release_to_pool in aiopg/psycopg3 (CQS)
- Rename misleading params in CalculateMetrics (pool→host, dsn→host)
- Add defensive copies in CalculateMetrics.metrics() and Dsn.params
- Modernize type annotations in utils.py (Optional→X|None, etc.)
- Simplify dead branch in PoolState.ready()
- Add __all__ to metrics module
- Update tests for new PoolAcquireContext constructor API
- Convert loop-assert to @pytest.mark.parametrize
- Use public property pool_state.driver in test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(lint): wrap long assertion line in test_acquire.py
a4ca62c 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(types): add type narrowing assert for replicas_count in pool_stat…
60a6454 
…e.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz
fix(acquire): propagate __aexit__ return value in TimeoutAcquireContext
fd4d1f3 
The missing `return` before `await self._context.__aexit__(*exc)` caused
the inner context's return value (e.g. `True` to suppress an exception)
to be silently discarded. Also drops the `-> None` annotation so the
actual `bool | None` return is accurately reflected.
@Pavkazzz @claude
fix(docs): convert doctest to code block in asyncsqlalchemy.py
1c970ac 
Replace doctest syntax (>>>) with RST code block (::) in the
async_sessionmaker docstring to prevent pytest --doctest-modules
from attempting to execute the example code.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(asyncpg): replace packaging dependency with tuple version comparison
8716eac 
Remove the fragile packaging import and replace it with a simple helper
function that parses asyncpg.__version__ into a tuple for comparison.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(asyncpg): replace WeakKeyDictionary with plain dict for host cache
097f2ee 
asyncpg.Pool uses __slots__ without __weakref__, so
weakref.WeakKeyDictionary cannot store pool references.
Use dict[int, str] keyed by id(pool) instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(docs): correct examples and references across docs
d9ff24d 
- class-scheme.md: fix import path to hasql.driver.*, fix
  PoolState description (is driver gateway, not state-only),
  remove nonexistent release()/terminate()/_refresh_pool_role,
  fix acquire path to use _register_connection, update import
  graph for acquire.py
- types.md: fix balancer policy constructor signature to use
  PoolStateProvider (not BasePoolManager), remove reference to
  nonexistent policy.py
- migration guide: fix test patching example to use
  _pool_state.driver, fix notify method name to
  notify_pool_checked, fix prepare_pool_factory_kwargs to
  return new dict instead of mutating

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(docs): update dead import paths in README.rst to hasql.driver.*
be90633 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(examples): update dead import paths to hasql.driver.*
81d2b3c 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(asyncpg): handle pre-release version strings in _asyncpg_version()
e06c0f9 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(balancer): use standard probability check for master_as_replica_w…
065abdd 
…eight

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
fix(psycopg3): stop passing unsupported kwargs to Psycopg3AcquireContext
e59efaf 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
refactor(balancer): use relative imports consistent with rest of code…
2b3a10a 
…base

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
feat: final validation — lint, tests, and dead import check all pass
4ae9768 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz @claude
docs: add stale replica detection design spec
c719d6f 
Spec for pluggable staleness detection that removes lagging replicas
from the active pool and falls back to them as a last resort.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Pavkazzz Pavkazzz closed this Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mosquito mosquito Awaiting requested review from mosquito

@hugiron hugiron Awaiting requested review from hugiron

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Pavkazzz