feat: Smart Retry Logic with Exponential Backoff (Issue #43) #278
Conversation
Implements Issue #43 - Smart Retry Logic with Exponential Backoff Features: - RetryConfig dataclass with configurable parameters - RetryManager class for executing operations with retry logic - Multiple backoff strategies: exponential, linear, constant, fibonacci - Configurable jitter to prevent thundering herd problem - @Retry decorator for easy function decoration - Preset configurations for network, API, and apt operations - Comprehensive test suite (33 tests) The module provides robust retry mechanisms for: - Network operations with transient failures - LLM API calls with rate limiting - Package manager operations with lock file issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughIntroduces a new retry module with configurable retry logic supporting multiple backoff strategies (exponential, linear, constant, Fibonacci) and jitter. Includes RetryManager class for executing retries, a decorator for easy function wrapping, preset configurations for common use cases (network, API, APT operations), and a comprehensive test suite validating all functionality. Changes
Sequence DiagramsequenceDiagram
actor User
participant RetryManager
participant TargetFunc as Target Function
participant Callback as on_retry Callback
User->>RetryManager: execute(func, args, kwargs)
loop Retry Attempts (max_attempts)
RetryManager->>TargetFunc: invoke with args/kwargs
alt Success
TargetFunc-->>RetryManager: result
Note over RetryManager: Return RetryResult (success=True)
else Exception Raised
TargetFunc-->>RetryManager: exception
alt Retryable Exception & Attempts Remaining
RetryManager->>RetryManager: _calculate_delay(attempt, strategy)
Note over RetryManager: Apply jitter if configured
RetryManager->>Callback: on_retry(attempt, exception, delay)
Callback-->>RetryManager: callback complete
Note over RetryManager: Wait delay seconds
Note over RetryManager: Collect error, increment attempt
else Non-Retryable or No Attempts Left
Note over RetryManager: Return RetryResult<br/>(success=False, final_error)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import random | ||
| import logging | ||
| import functools | ||
| from typing import Callable, TypeVar, Optional, Tuple, Type, Union, List |
| """ | ||
|
|
||
| import pytest | ||
| import time |
|
|
||
| import pytest | ||
| import time | ||
| from unittest.mock import Mock, patch, call |
| from cortex.retry import ( | ||
| RetryConfig, | ||
| RetryStrategy, | ||
| RetryResult, | ||
| RetryManager, | ||
| retry, | ||
| NETWORK_RETRY_CONFIG, | ||
| API_RETRY_CONFIG, | ||
| APT_RETRY_CONFIG, | ||
| retry_apt_operation, | ||
| retry_api_call, | ||
| retry_network_operation, | ||
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
cortex/retry.py (3)
53-61: Consider validatingexponential_base.The validation logic is thorough, but
exponential_baseis not validated. A value ≤ 0 would cause issues (negative/zero delays or math errors), and a value < 1 would cause decreasing delays instead of increasing backoff.def __post_init__(self): if self.max_attempts < 1: raise ValueError("max_attempts must be at least 1") if self.base_delay < 0: raise ValueError("base_delay must be non-negative") if self.max_delay < self.base_delay: raise ValueError("max_delay must be >= base_delay") if not 0 <= self.jitter_range <= 1: raise ValueError("jitter_range must be between 0 and 1") + if self.exponential_base <= 0: + raise ValueError("exponential_base must be positive")
87-88: Annotate class constant withClassVarand prefer tuple for immutability.The
_FIBONACCIsequence is a class-level constant that should be annotated withClassVarand made immutable.+from typing import ClassVar + class RetryManager: """Manages retry operations with configurable backoff strategies.""" # Precomputed Fibonacci sequence for fibonacci backoff - _FIBONACCI = [1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144] + _FIBONACCI: ClassVar[tuple[int, ...]] = (1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144)
179-183: Uselogging.exceptionto preserve the stack trace.When logging the final failure,
logging.exceptionautomatically includes the traceback, which aids debugging.else: - logger.error( + logger.exception( f"All {self.config.max_attempts} attempts failed. " f"Final error: {e}" )tests/test_retry.py (1)
350-375: Consider adding retry scenario tests for convenience functions.The convenience function tests only cover the immediate success case. Consider adding at least one test that verifies the retry behavior through these helpers (e.g., a mock that fails once then succeeds) to ensure the preset configs are applied correctly.
def test_retry_network_operation_with_retries(self): """Test retry_network_operation actually retries on failure.""" mock_func = Mock(side_effect=[ConnectionError("fail"), b"data"]) result = retry_network_operation(mock_func) assert result.success is True assert result.attempts == 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cortex/retry.py(1 hunks)tests/test_retry.py(1 hunks)
🧰 Additional context used
🪛 GitHub Check: SonarCloud Code Analysis
tests/test_retry.py
[warning] 340-340: Do not perform equality checks with floating point values.
[warning] 176-176: Do not perform equality checks with floating point values.
[warning] 162-162: Do not perform equality checks with floating point values.
[warning] 181-181: Do not perform equality checks with floating point values.
[warning] 48-48: Do not perform equality checks with floating point values.
[warning] 334-334: Do not perform equality checks with floating point values.
[warning] 130-130: Do not perform equality checks with floating point values.
[warning] 195-195: Do not perform equality checks with floating point values.
[warning] 333-333: Do not perform equality checks with floating point values.
[warning] 34-34: Do not perform equality checks with floating point values.
[warning] 132-132: Do not perform equality checks with floating point values.
[warning] 179-179: Do not perform equality checks with floating point values.
[warning] 161-161: Do not perform equality checks with floating point values.
[warning] 49-49: Do not perform equality checks with floating point values.
[warning] 341-341: Do not perform equality checks with floating point values.
[warning] 149-149: Do not perform equality checks with floating point values.
[warning] 148-148: Do not perform equality checks with floating point values.
[warning] 131-131: Do not perform equality checks with floating point values.
[warning] 346-346: Do not perform equality checks with floating point values.
[warning] 33-33: Do not perform equality checks with floating point values.
[warning] 32-32: Do not perform equality checks with floating point values.
[warning] 133-133: Do not perform equality checks with floating point values.
[warning] 177-177: Do not perform equality checks with floating point values.
[warning] 163-163: Do not perform equality checks with floating point values.
[warning] 414-414: Replace this generic exception class with a more specific one.
[warning] 36-36: Do not perform equality checks with floating point values.
[warning] 178-178: Do not perform equality checks with floating point values.
[warning] 147-147: Do not perform equality checks with floating point values.
[warning] 180-180: Do not perform equality checks with floating point values.
[warning] 146-146: Do not perform equality checks with floating point values.
🪛 Ruff (0.14.8)
cortex/retry.py
55-55: Avoid specifying long messages outside the exception class
(TRY003)
57-57: Avoid specifying long messages outside the exception class
(TRY003)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
61-61: Avoid specifying long messages outside the exception class
(TRY003)
88-88: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
126-126: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
180-183: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
tests/test_retry.py
292-292: Avoid specifying long messages outside the exception class
(TRY003)
302-302: Avoid specifying long messages outside the exception class
(TRY003)
389-389: Avoid specifying long messages outside the exception class
(TRY003)
414-414: Create your own exception
(TRY002)
414-414: Avoid specifying long messages outside the exception class
(TRY003)
428-428: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
cortex/retry.py (2)
44-44: Verify defaultmax_attemptsagainst issue requirements.Issue #43 specifies "Maximum retry attempts configurable (default specified as 5)", but the default here is 3. Please confirm whether this is intentional or should be updated to match the specification.
250-273: LGTM!The preset configurations are well-designed for their respective use cases, with appropriate tuning of delays and jitter settings.
tests/test_retry.py (3)
25-70: LGTM!Comprehensive validation tests covering all edge cases in
RetryConfig.__post_init__. The floating-point equality checks are safe here since jitter is disabled and values are deterministic.
117-181: LGTM!Thorough delay calculation tests covering all four backoff strategies with deterministic assertions.
378-434: LGTM!Solid integration tests covering realistic retry scenarios including transient failures, rate limiting, and permanent failures.
Summary
Implements Issue #43: Smart Retry Logic with Exponential Backoff
This PR adds a comprehensive retry module (
cortex/retry.py) that provides robust retry mechanisms for operations that may fail transiently.Features
Usage Examples
Test Plan
Files Changed
cortex/retry.py- New retry module (320 lines)tests/test_retry.py- Comprehensive test suite (441 lines)Closes #43
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.