Skip to content

feat(client): configurable timeout and retry settings (Closes #5)#31

Open
David-patrick-chuks wants to merge 1 commit into
ShadeProtocol:mainfrom
David-patrick-chuks:fix/5-configurable-timeout-retry
Open

feat(client): configurable timeout and retry settings (Closes #5)#31
David-patrick-chuks wants to merge 1 commit into
ShadeProtocol:mainfrom
David-patrick-chuks:fix/5-configurable-timeout-retry

Conversation

@David-patrick-chuks

@David-patrick-chuks David-patrick-chuks commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #5

Adds global and per-instance timeout/retry configuration for ShadeClient (Gateway):

  • shade.timeout and shade.max_retries module-level settings backed by config.py
  • Per-client overrides via ShadeClient(timeout=..., max_retries=...)
  • Validation: timeout > 0 and 0 <= max_retries <= 10
  • Settings flow through to SyncHTTPClient / AsyncHTTPClient transport layers

Acceptance criteria

Criteria Status
ShadeClient(timeout=5.0) times out after 5 seconds
ShadeClient(max_retries=0) disables retries
Negative/unreasonably large values raise ValueError
Global shade.timeout / shade.max_retries respected

Test plan

  • poetry run pytest -v — 81/81 passed

Summary by CodeRabbit

  • New Features

    • Added configurable timeout and max_retries settings at the module level and per client.
    • Client creation now supports overriding these values directly, with module defaults used when omitted.
  • Bug Fixes

    • Added validation for timeout and retry values to prevent invalid configurations.
    • Retry behavior can now be disabled by setting retries to 0, and timeout settings are passed through consistently.

Add global shade.timeout and shade.max_retries backed by config.py,
validate settings on client construction, and resolve per-instance
ShadeClient/Gateway overrides with module-level defaults.

Closes ShadeProtocol#5
@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The package adds shared timeout and retry settings with defaults and validation. shade exports config-backed timeout and max_retries, Gateway and HTTP clients use those values when omitted, and new tests cover validation and runtime behavior.

Changes

Configurable client timeout and retry settings

Layer / File(s) Summary
Shared settings contract
src/shade/config.py, src/shade/__init__.py
Default timeout and retry values plus validation are added, and the package module now exposes timeout and max_retries as config-backed attributes.
Gateway and HTTP client wiring
src/shade/gateway.py, src/shade/http.py
Gateway and both HTTP clients accept optional timeout and retry inputs, resolve missing values from module config, validate the resolved settings, and pass them into client construction.
Client settings tests
tests/test_client_settings.py
Tests cover validation boundaries, module-level assignments, per-client overrides, timeout pass-through, retry disablement at zero retries, and the ShadeClient alias.

Sequence Diagram(s)

sequenceDiagram
  participant shade
  participant config
  participant validate_client_settings
  participant Gateway
  participant SyncHTTPClient
  participant AsyncHTTPClient

  shade->>config: set timeout and max_retries
  Gateway->>config: read defaults when arguments are None
  Gateway->>validate_client_settings: validate resolved timeout and max_retries
  Gateway->>SyncHTTPClient: pass resolved timeout and max_retries
  Gateway->>AsyncHTTPClient: pass resolved timeout and max_retries
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • codebestia

Poem

🐇 I nibbled the clock and set retries just so,
shade.timeout whispers, “take it slow.”
When 429s hop by, I pause, then flee,
with max_retries=0, the burrow stays free.
Thump-thump, the client skips in sync with me.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the configurable timeout and retry settings change and matches the PR scope.
Description check ✅ Passed The description includes the summary, issue reference, acceptance criteria, and test plan; only optional template sections are omitted.
Linked Issues check ✅ Passed The changes implement global and per-instance timeout/retry settings, validation, and transport wiring required by #5.
Out of Scope Changes check ✅ Passed The modified files and tests all support the timeout/retry feature, with no obvious unrelated scope added.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/shade/__init__.py (1)

60-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider validating in the timeout/max_retries setters.

These setters write straight to _config without validation, so shade.timeout = 0 or shade.max_retries = -1 succeeds silently and only fails later when a client is constructed. Validating at assignment time gives the user a clear, immediate error at the point of the mistake.

♻️ Validate on assignment
     `@timeout.setter`
     def timeout(self, value: float) -> None:
         from . import config as _config
+        _config.validate_client_settings(value, _config.max_retries)
         _config.timeout = value
@@
     `@max_retries.setter`
     def max_retries(self, value: int) -> None:
         from . import config as _config
+        _config.validate_client_settings(_config.timeout, value)
         _config.max_retries = value
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/shade/__init__.py` around lines 60 - 73, The timeout and max_retries
setters in shade.__init__.py currently assign directly to _config without any
guard, so invalid values can slip through until client creation; add
assignment-time validation in the timeout and max_retries property setters to
reject non-positive timeout values and negative retry counts with an immediate,
clear error. Use the existing timeout, max_retries, and config property setters
as the place to enforce the checks so invalid values fail at the point of
assignment rather than later.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/shade/__init__.py`:
- Around line 60-73: The timeout and max_retries setters in shade.__init__.py
currently assign directly to _config without any guard, so invalid values can
slip through until client creation; add assignment-time validation in the
timeout and max_retries property setters to reject non-positive timeout values
and negative retry counts with an immediate, clear error. Use the existing
timeout, max_retries, and config property setters as the place to enforce the
checks so invalid values fail at the point of assignment rather than later.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2344b7c1-32c5-4678-a837-fa80bd53f223

📥 Commits

Reviewing files that changed from the base of the PR and between 3ad7931 and 8c9ea7c.

📒 Files selected for processing (5)
  • src/shade/__init__.py
  • src/shade/config.py
  • src/shade/gateway.py
  • src/shade/http.py
  • tests/test_client_settings.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement configurable timeout and retry settings on client

1 participant