feat: per-endpoint circuit breaker option#22
Merged
Conversation
Add `per_endpoint_circuit_breaker: bool = False` to `HyperpingClient` and `AsyncHyperpingClient`. When enabled, the client maintains an independent `CircuitBreaker` per request path (query string and fragment stripped from the key) so a single flaky endpoint no longer blocks traffic to healthy ones. Default `False` preserves the original single-shared-breaker behaviour and all existing breaker tests. Per-path state is queryable via `client.circuit_breaker_state_for(path)`; the existing `client.circuit_breaker` property still returns the shared breaker. The per-path dict is protected by its own `threading.Lock`; the per-path `CircuitBreaker` instances retain their own internal lock. Tests cover: isolation between paths, state-query API, query-string stripping, default-mode unchanged, async parity, and 50-thread concurrent access.
Address review feedback on the initial per-endpoint breaker implementation.
The previous version keyed the breaker dict on the literal request path. For
paths that embed resource UUIDs (`/v1/monitors/{uuid}`, etc.) this meant
every resource ended up with its own breaker, the dict could grow unbounded
in long-lived clients, and the README's `circuit_breaker_state_for(Endpoint.MONITORS)`
example only saw list-call state, not per-resource calls.
Changes:
- Default breaker key now collapses sub-resource paths under their matching
`Endpoint` prefix (`/v1/monitors`, `/v3/incidents`, ...). The breaker set is
bounded by the number of Endpoint values, not by resource cardinality.
- New `breaker_key_fn: Callable[[str], str] | None` lets callers opt into a
different scheme (per-UUID, per-verb, etc.) and explicitly own bounding.
- OPEN-state error message in per-endpoint mode now includes the breaker key
("Circuit breaker OPEN for '/v1/monitors' - ..."), so the failure points at
which endpoint tripped.
- `circuit_breaker_state_for(path)` now falls back to the shared breaker's
state in default mode instead of raising, so the call is always safe to
make and toggling the flag at construction time doesn't change the API
surface callers see.
- `_breaker_key` rewritten with `urllib.parse.urlsplit` and the dict-lock
comment explains the `threading.Lock` choice for the async client.
Tests added for endpoint canonicalisation, custom `breaker_key_fn`, default-
mode `circuit_breaker_state_for` returning shared state, and the OPEN error
message including the endpoint key.
3 tasks
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
1 participant
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.
Summary
per_endpoint_circuit_breaker: bool = FalseonHyperpingClientandAsyncHyperpingClient. When enabled, each request path has its ownCircuitBreakerinstance, so one flaking endpoint no longer trips the breaker for traffic to healthy endpoints.client.circuit_breaker_state_for(path). Query string and fragment are stripped from the key so/v3/incidentsand/v3/incidents?status=openshare state. The existingclient.circuit_breakerproperty continues to return the shared breaker.Why
The previous single-shared breaker design had a known failure mode (M3 in
BACKLOG.md): if any one endpoint started returning 5xx repeatedly, every other endpoint was also blocked once the failure threshold was reached. Reads on healthy endpoints were punished alongside the flaking one. This change lets users opt into per-path isolation when that trade-off doesn't fit their workload.Implementation notes
dict[str, CircuitBreaker]keyed by path, lazily populated on first request.threading.Lock; eachCircuitBreakerkeeps its own internal lock for state mutations.circuit_breaker_configis reused for every per-path breaker (no per-path tuning, by design — keeps the API simple).README.md;CHANGELOG.md[Unreleased]section added;BACKLOG.mdM3 marked done.Test plan
tests/unit/test_per_endpoint_circuit_breaker.py— 7 new tests: isolation, per-path state query, query-string stripping, default behaviour unchanged, error on misuse, async parity, 50-thread concurrent accessclient.py100%,_async_client.py91%,_circuit_breaker.py94%ruff check,ruff format,mypy srcall clean