Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vale/styles/spelling-exceptions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ artifact_definitions
artifact_name
async
Authentik
backoff
boolean
check_definitions
class_name
Expand Down
3 changes: 2 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@

<!-- SPECKIT START -->
For additional context about technologies to be used, project structure,
shell commands, and other important information, read the current plan
shell commands, and other important information, read the current plan:
`specs/ihs-249-sdk-429-retry/plan.md`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be part of the PR

<!-- SPECKIT END -->
1 change: 1 addition & 0 deletions changelog/1124.added.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably only be one of these changelogs

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added transparent retry of HTTP 429 (rate-limited) responses on both `InfrahubClient` and `InfrahubClientSync`. Retries use jittered exponential backoff and honour a server-provided `Retry-After` header (delta-seconds or HTTP-date). The behaviour is tunable through four new `Config` fields (`rate_limit_retry_enabled`, `rate_limit_max_retries`, `rate_limit_backoff_base`, `rate_limit_backoff_max`), and a new `RateLimitError` exception is raised when retries are exhausted.
1 change: 1 addition & 0 deletions changelog/1124.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A persistent HTTP 429 (rate-limited) response now raises `RateLimitError` once the configured retries are exhausted, instead of surfacing the raw `httpx.HTTPStatusError`. The underlying `httpx.HTTPStatusError` remains available via the exception's `__cause__`.
43 changes: 43 additions & 0 deletions dev/specs/ihs-249-sdk-429-retry/alignment-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Spec/Ask Alignment Check: SDK retry with backoff on HTTP 429 responses

**Date**: 2026-07-07
**Feature**: [spec.md](./spec.md)

## 1. Source

**Source PRD**: Jira IHS-249 — "SDK retry with backoff on HTTP 429 responses"
(`https://opsmill.atlassian.net/browse/IHS-249`), fetched via the Atlassian MCP tool.
The issue body is itself a full, structured PRD (Problem Statement, Solution Overview, 9 User
Stories, 3 prioritised User Journeys with acceptance criteria, FR-001…009, Key Entities, Edge
Cases, SC-001…005, Implementation/Testing Decisions, Out of Scope, one Open Question). Related
GitHub issue: opsmill/infrahub-sdk-python#1124. No secondary URLs to fetch.

## 2. Verdict

Result: ✅ ALIGNED

`spec.md` faithfully carries every PRD requirement, acceptance criterion, and scope boundary.
The only additions are an expansion of an existing requirement and the authorized resolution of
the PRD's explicit open question — neither is drift under the check's definition.

## 3. Findings

| Severity | Category | PRD reference | Spec reference | Description |
| ---------- | ---------- | --------------- | ---------------- | ------------- |
| ✅ none | missing | FR-001…009 | FR-001…009 | All nine functional requirements present, none dropped or softened (attempt cap, jittered+clamped backoff, Retry-After both forms, malformed fallback, RateLimitError with url/attempts/last-Retry-After, all request paths, per-retry logging, async/sync parity, tune+disable). |
| ✅ none | missing | Journeys P1–P3, User Stories 1–9 | US1–US4, Edge Cases | P1/P2/P3 journeys map to US1/US2/US3; PRD user story 8 (tune/disable) surfaced as US4. All acceptance scenarios preserved. |
| ✅ none | missing | SC-001…005 | SC-001…005 | Success criteria carried over with equivalent semantics. |
| ✅ none | contradicted | Out of Scope (503, server-side INFP-636/635, `retry_on_failure`) | Out of Scope | Scope boundaries reproduced verbatim; nothing contradicted. |
| ℹ️ info | added (authorized) | Open Question (chain httpx.HTTPStatusError as `__cause__`?) | FR-005, Assumptions | The PRD's single open question was resolved affirmatively (chain the transport error as `__cause__`). The parent prep flow explicitly authorizes autonomous clarification resolution; recorded as an assumption. Not drift. |
| ℹ️ info | added (derived) | FR-009 (disable via Config) | SC-006 | Spec adds SC-006 (disabled path raises immediately). This is a measurable expansion of FR-009, not new scope. |
| ℹ️ info | added (design) | Assumption: single `_request` chokepoint | plan.md R1 / data-model | Plan (not spec) records that multipart/streaming bypass `_request`, so retry is applied at three sites. This corrects a PRD *assumption* at the implementation layer while still satisfying FR-006; spec requirements unchanged. Not spec drift. |

No requirements are missing, no acceptance criteria dropped or softened, no requirement semantics
changed, and no off-scope scope items were introduced. The Config field defaults (enabled, 5, 0.5,
60), the new `RateLimitError`, and the additive-only API surface all match the PRD exactly.

## 4. Action

**Proceed.** No remediation passes required (remediation counter: 0). `tasks.md` is safe to hand to
the implementation phase. The affirmative resolution of the open question and the SC-006 derivation
are documented above for traceability.
39 changes: 39 additions & 0 deletions dev/specs/ihs-249-sdk-429-retry/checklists/requirements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Specification Quality Checklist: SDK retry with backoff on HTTP 429 responses

**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-07-07
**Feature**: [spec.md](../spec.md)

## Content Quality

- [x] No implementation details (languages, frameworks, APIs)
- [x] Focused on user value and business needs
- [x] Written for non-technical stakeholders
- [x] All mandatory sections completed

## Requirement Completeness

- [x] No [NEEDS CLARIFICATION] markers remain
- [x] Requirements are testable and unambiguous
- [x] Success criteria are measurable
- [x] Success criteria are technology-agnostic (no implementation details)
- [x] All acceptance scenarios are defined
- [x] Edge cases are identified
- [x] Scope is clearly bounded
- [x] Dependencies and assumptions identified

## Feature Readiness

- [x] All functional requirements have clear acceptance criteria
- [x] User scenarios cover primary flows
- [x] Feature meets measurable outcomes defined in Success Criteria
- [x] No implementation details leak into specification

## Notes

- The PRD's single open question (whether the exhaustion error should chain the
underlying transport error as its cause) was resolved affirmatively and encoded
into FR-005 and the Assumptions section, so no [NEEDS CLARIFICATION] markers remain.
- Entity names in the spec are described in capability terms (e.g. "rate-limit retry
decision logic") rather than concrete class names to keep the spec implementation-agnostic;
concrete names (`RateLimitRetryHandler`, `RateLimitError`, `Config` fields) are deferred to plan.md.
37 changes: 37 additions & 0 deletions dev/specs/ihs-249-sdk-429-retry/contracts/config.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Contract: `Config` rate-limit fields (additive, public)

Added to `infrahub_sdk/config.py::ConfigBase`. All additive; no existing field changes.

```python
rate_limit_retry_enabled: bool = Field(
default=True,
description="Retry requests that receive HTTP 429 using backoff. Set False to disable.",
)
rate_limit_max_retries: int = Field(
default=5,
ge=0,
description="Maximum number of retries after the initial attempt when receiving HTTP 429.",
)
rate_limit_backoff_base: float = Field(
default=0.5,
gt=0,
description="Base interval in seconds for exponential backoff between 429 retries.",
)
rate_limit_backoff_max: float = Field(
default=60.0,
gt=0,
description="Maximum wait in seconds for any single 429 retry (also clamps Retry-After).",
)
```

## Backward compatibility

- Purely additive; existing code constructing `Config(...)` / `InfrahubClient(...)` is unaffected.
- Environment-variable overrides follow the existing `BaseSettings` mechanism (e.g.
`INFRAHUB_RATE_LIMIT_MAX_RETRIES`), consistent with current fields.

## Guarantees

- `rate_limit_retry_enabled=False` ⇒ a 429 is returned/raised exactly as before this feature
(no wait, no extra attempt). (FR-009, SC-006)
- Defaults produce transparent retry for typical background workloads. (FR-001)
41 changes: 41 additions & 0 deletions dev/specs/ihs-249-sdk-429-retry/contracts/rate_limit_error.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Contract: `RateLimitError` (new public exception)

Added to `infrahub_sdk/exceptions.py`. Subclass of the base `Error`.

```python
class RateLimitError(Error):
def __init__(
self,
url: str,
attempts: int,
retry_after: float | None = None,
message: str | None = None,
) -> None:
self.url = url
self.attempts = attempts
self.retry_after = retry_after
if message is None:
message = (
f"Request to {url} was rate-limited (HTTP 429) after {attempts} attempt(s)."
)
super().__init__(message)
```

## Contract

- **Raised**: by the client retry driver when 429s persist past `rate_limit_max_retries`. (FR-005)
- **Type**: `isinstance(err, Error)` is `True` — callers catching the SDK base `Error` still catch it.
- **Distinct**: it is NOT an `httpx.HTTPStatusError`; callers can `except RateLimitError` to
distinguish rate-limit exhaustion from other HTTP failures. (User story 6)
- **Attributes**: `url: str`, `attempts: int` (= `max_retries + 1`), `retry_after: float | None`
(last observed `Retry-After` in seconds, `None` if never present or unparseable).
- **Cause chaining**: raised with `raise RateLimitError(...) from http_status_error`, so
`err.__cause__` is the underlying `httpx.HTTPStatusError` built from the final 429 response.
Callers can inspect `err.__cause__.response` for the raw response. (Open-question resolution)

## Behavioural change (changelog callout)

Before this feature, a persistent 429 surfaced as `httpx.HTTPStatusError` (via
`raise_for_status()`). With retry enabled (default), it now surfaces as `RateLimitError`
after exhaustion. Callers relying on catching `httpx.HTTPStatusError` for 429 should either
catch `RateLimitError` or inspect `__cause__`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Contract: `RateLimitRetryHandler` (new, pure logic)

New module `infrahub_sdk/rate_limit.py`. No I/O, no sleeping — deterministic and unit-testable.

```python
class RateLimitRetryHandler:
def __init__(self, max_retries: int, backoff_base: float, backoff_max: float) -> None: ...

def parse_retry_after(
self, header: str | None, *, now: datetime | None = None
) -> float | None:
"""Return seconds to wait per Retry-After, or None if absent/malformed.
- delta-seconds: int(header)
- HTTP-date: (parsedate_to_datetime(header) - now).total_seconds(), floored at 0
- anything unparseable: None (caller falls back to computed backoff)."""

def compute_backoff(self, attempt: int) -> float:
"""Deterministic exponential ceiling: min(backoff_max, backoff_base * 2**attempt)."""

def jittered_delay(self, ceiling: float) -> float:
"""Full jitter: random.uniform(0, ceiling)."""

def next_delay(
self, attempt: int, retry_after_header: str | None = None, *, now: datetime | None = None
) -> float:
"""Honour Retry-After if parseable (clamped to backoff_max), else jittered backoff."""

def should_retry(self, attempts_made: int) -> bool:
"""True while retries remain: attempts_made <= max_retries."""
```

## Contract guarantees (map to FR / SC)

- `compute_backoff` is monotonic non-decreasing in `attempt` and never exceeds `backoff_max`. (FR-002, SC-003)
- `jittered_delay(c)``[0, c]`; two calls (or two handler instances) are extremely unlikely to
match, satisfying "differ between instances". Tests assert jitter by sampling. (SC-003)
- `next_delay` clamps every result — computed *and* `Retry-After` — to `backoff_max`. (FR-003)
- `parse_retry_after` never raises on bad input; returns `None`. (FR-004)
- Past HTTP-date ⇒ `parse_retry_after` returns `0.0`, never negative. (Edge case)
- `should_retry` yields exactly `max_retries` retries ⇒ `max_retries + 1` total sends. (FR-001, SC-004)

## Determinism for tests

- `parse_retry_after`/`next_delay` accept an injectable `now` for HTTP-date tests.
- Jitter is the only nondeterministic element; tests either assert on `compute_backoff`
(deterministic ceiling) or assert `0 <= jittered_delay(c) <= c` and that a sample of draws varies.
Loading