Skip to content

Commit 93241de

Browse files
committed
fix(retry): align REST max_retries to count retries not total attempts
REST `_RetryTransport` and `_AsyncRetryTransport` previously looped `range(max_retries)`, making `max_retries` the *total* attempt count (initial + retries). gRPC counts retries only, so the same `RetryConfig(max_retries=N)` gave different behavior per transport. Change the loops to `range(max_retries + 1)` and update the last-attempt guards from `attempt < max_retries - 1` to `attempt < max_retries` so both transports treat `max_retries` as the number of retries after the initial attempt. Also update the `RetryConfig.max_retries` docstring and all unit tests that hard-coded expected call counts under the old semantics.
1 parent f23ceeb commit 93241de

8 files changed

Lines changed: 41 additions & 30 deletions

File tree

pinecone/_internal/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class RetryConfig:
7979
"""Configuration for HTTP retry behavior.
8080
8181
Args:
82-
max_retries: Total number of request attempts (initial + retries). Defaults to 3.
82+
max_retries: Number of retries after the initial attempt. Defaults to 3 (4 total attempts).
8383
backoff_factor: Exponential backoff base multiplier in seconds. Defaults to 2.0.
8484
max_wait: Maximum backoff delay in seconds. Defaults to 60.0.
8585
retryable_status_codes: HTTP status codes that trigger a retry. Defaults to

pinecone/_internal/http_client.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,16 @@ def __init__(
131131

132132
def handle_request(self, request: httpx.Request) -> httpx.Response:
133133
last_exc: httpx.TransportError | None = None
134-
for attempt in range(self._config.max_retries):
134+
for attempt in range(self._config.max_retries + 1):
135135
try:
136136
response = self._transport.handle_request(request)
137137
except httpx.TransportError as exc:
138138
last_exc = exc
139-
if attempt < self._config.max_retries - 1:
139+
if attempt < self._config.max_retries:
140140
logger.debug(
141141
"Connection error on attempt %d/%d, retrying: %s",
142142
attempt + 1,
143-
self._config.max_retries,
143+
self._config.max_retries + 1,
144144
exc,
145145
)
146146
time.sleep(self._compute_backoff(attempt))
@@ -149,7 +149,7 @@ def handle_request(self, request: httpx.Request) -> httpx.Response:
149149
if response.status_code not in self._config.retryable_status_codes:
150150
return response
151151
self._notify_throttle(request)
152-
if attempt < self._config.max_retries - 1:
152+
if attempt < self._config.max_retries:
153153
response.close()
154154
retry_after = response.headers.get("retry-after")
155155
if retry_after is not None:
@@ -201,16 +201,16 @@ def __init__(
201201

202202
async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
203203
last_exc: httpx.TransportError | None = None
204-
for attempt in range(self._config.max_retries):
204+
for attempt in range(self._config.max_retries + 1):
205205
try:
206206
response = await self._transport.handle_async_request(request)
207207
except httpx.TransportError as exc:
208208
last_exc = exc
209-
if attempt < self._config.max_retries - 1:
209+
if attempt < self._config.max_retries:
210210
logger.debug(
211211
"Connection error on attempt %d/%d, retrying: %s",
212212
attempt + 1,
213-
self._config.max_retries,
213+
self._config.max_retries + 1,
214214
exc,
215215
)
216216
await asyncio.sleep(self._compute_backoff(attempt))
@@ -219,7 +219,7 @@ async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
219219
if response.status_code not in self._config.retryable_status_codes:
220220
return response
221221
self._notify_throttle(request)
222-
if attempt < self._config.max_retries - 1:
222+
if attempt < self._config.max_retries:
223223
await response.aclose()
224224
retry_after = response.headers.get("retry-after")
225225
if retry_after is not None:

tests/unit/_internal/test_retry_transport.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ def test_connection_error_is_retried_and_succeeds() -> None:
3333

3434

3535
def test_connection_error_exhausts_retries_and_raises() -> None:
36-
rt, inner = _transport(max_retries=2)
36+
# max_retries=1 → 2 total attempts (initial + 1 retry)
37+
rt, inner = _transport(max_retries=1)
3738
inner.handle_request.side_effect = [
3839
httpx.RemoteProtocolError("peer closed connection"),
3940
httpx.RemoteProtocolError("peer closed connection"),
@@ -80,7 +81,8 @@ async def test_async_connection_error_is_retried_and_succeeds() -> None:
8081

8182
@pytest.mark.asyncio
8283
async def test_async_connection_error_exhausts_retries_and_raises() -> None:
83-
rt, inner = _async_transport(max_retries=2)
84+
# max_retries=1 → 2 total attempts (initial + 1 retry)
85+
rt, inner = _async_transport(max_retries=1)
8486
inner.handle_async_request.side_effect = [
8587
httpx.RemoteProtocolError("peer closed connection"),
8688
httpx.RemoteProtocolError("peer closed connection"),
@@ -158,7 +160,8 @@ def test_post_not_retried_on_400() -> None:
158160

159161

160162
def test_post_exhausts_retries_then_returns_503() -> None:
161-
rt, inner = _transport(max_retries=3)
163+
# max_retries=2 → 3 total attempts (initial + 2 retries)
164+
rt, inner = _transport(max_retries=2)
162165
inner.handle_request.side_effect = [
163166
httpx.Response(503),
164167
httpx.Response(503),
@@ -170,7 +173,8 @@ def test_post_exhausts_retries_then_returns_503() -> None:
170173

171174

172175
def test_post_exhausts_retries_then_raises_transport_error() -> None:
173-
rt, inner = _transport(max_retries=3)
176+
# max_retries=2 → 3 total attempts (initial + 2 retries)
177+
rt, inner = _transport(max_retries=2)
174178
inner.handle_request.side_effect = [
175179
httpx.ConnectError("fail"),
176180
httpx.ConnectError("fail"),

tests/unit/preview/test_async_indexes_exists.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def indexes() -> AsyncPreviewIndexes:
4545
config = PineconeConfig(
4646
api_key="test-key",
4747
host=BASE_URL,
48-
retry_config=RetryConfig(max_retries=1),
48+
retry_config=RetryConfig(max_retries=0),
4949
)
5050
return AsyncPreviewIndexes(config=config)
5151

tests/unit/preview/test_indexes_exists.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def indexes() -> PreviewIndexes:
4444
config = PineconeConfig(
4545
api_key="test-key",
4646
host=BASE_URL,
47-
retry_config=RetryConfig(max_retries=1),
47+
retry_config=RetryConfig(max_retries=0),
4848
)
4949
return PreviewIndexes(config=config)
5050

tests/unit/test_http_client.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,8 @@ def test_on_throttle_called_on_429(self, monkeypatch: pytest.MonkeyPatch) -> Non
805805
def test_on_throttle_called_each_retry(self, monkeypatch: pytest.MonkeyPatch) -> None:
806806
monkeypatch.setattr("pinecone._internal.http_client.time.sleep", lambda _: None)
807807
mock_callback = MagicMock()
808-
config = RetryConfig(max_retries=3, on_throttle=mock_callback)
808+
# max_retries=2 → 3 total attempts (initial + 2 retries)
809+
config = RetryConfig(max_retries=2, on_throttle=mock_callback)
809810
inner = _SequencedTransport([httpx.Response(429), httpx.Response(429), httpx.Response(429)])
810811
transport = _RetryTransport(transport=inner, retry_config=config) # type: ignore[arg-type]
811812
request = httpx.Request("GET", "https://api.pinecone.io/indexes")

tests/unit/test_retry.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,10 @@ def test_sync_exhausts_retries(self, mock_sleep: Any) -> None:
204204
)
205205
transport = _RetryTransport(
206206
transport=fake, # type: ignore[arg-type]
207-
retry_config=RetryConfig(max_retries=5, backoff_factor=2.0, max_wait=0.01),
207+
# max_retries=4 → 5 total attempts (initial + 4 retries)
208+
retry_config=RetryConfig(max_retries=4, backoff_factor=2.0, max_wait=0.01),
208209
)
209210
response = transport.handle_request(_make_request())
210-
# max_retries=5 total, so 5 calls
211211
assert response.status_code == 503
212212
assert fake.call_count == 5
213213
assert mock_sleep.call_count == 4
@@ -327,8 +327,9 @@ def test_sync_backoff_capped(self, mock_sleep: Any, mock_uniform: Any) -> None:
327327
)
328328
transport = _RetryTransport(
329329
transport=fake, # type: ignore[arg-type]
330+
# max_retries=4 → 5 total attempts (initial + 4 retries) = 4 sleeps
330331
retry_config=RetryConfig(
331-
max_retries=5,
332+
max_retries=4,
332333
backoff_factor=2.0,
333334
max_wait=3.0,
334335
),
@@ -516,7 +517,8 @@ def test_sync_closes_all_on_exhaustion(self, mock_sleep: Any) -> None:
516517
fake = _TrackingTransport(responses)
517518
transport = _RetryTransport(
518519
transport=fake, # type: ignore[arg-type]
519-
retry_config=RetryConfig(max_retries=3, backoff_factor=2.0, max_wait=0.01),
520+
# max_retries=2 → 3 total attempts (initial + 2 retries)
521+
retry_config=RetryConfig(max_retries=2, backoff_factor=2.0, max_wait=0.01),
520522
)
521523
result = transport.handle_request(_make_request())
522524
assert result.status_code == 500
@@ -597,10 +599,10 @@ async def test_async_exhausts_retries(self, mock_sleep: Any) -> None:
597599
)
598600
transport = _AsyncRetryTransport(
599601
transport=fake, # type: ignore[arg-type]
600-
retry_config=RetryConfig(max_retries=5, backoff_factor=2.0, max_wait=0.01),
602+
# max_retries=4 → 5 total attempts (initial + 4 retries)
603+
retry_config=RetryConfig(max_retries=4, backoff_factor=2.0, max_wait=0.01),
601604
)
602605
response = await transport.handle_async_request(_make_request())
603-
# max_retries=5 total, so 5 calls
604606
assert response.status_code == 502
605607
assert fake.call_count == 5
606608
assert mock_sleep.call_count == 4
@@ -659,8 +661,9 @@ async def test_async_backoff_capped(self, mock_sleep: Any, mock_uniform: Any) ->
659661
)
660662
transport = _AsyncRetryTransport(
661663
transport=fake, # type: ignore[arg-type]
664+
# max_retries=4 → 5 total attempts (initial + 4 retries) = 4 sleeps
662665
retry_config=RetryConfig(
663-
max_retries=5,
666+
max_retries=4,
664667
backoff_factor=2.0,
665668
max_wait=3.0,
666669
),
@@ -704,7 +707,8 @@ async def test_async_closes_all_on_exhaustion(self, mock_sleep: Any) -> None:
704707
fake = _TrackingAsyncTransport(responses)
705708
transport = _AsyncRetryTransport(
706709
transport=fake, # type: ignore[arg-type]
707-
retry_config=RetryConfig(max_retries=3, backoff_factor=2.0, max_wait=0.01),
710+
# max_retries=2 → 3 total attempts (initial + 2 retries)
711+
retry_config=RetryConfig(max_retries=2, backoff_factor=2.0, max_wait=0.01),
708712
)
709713
result = await transport.handle_async_request(_make_request())
710714
assert result.status_code == 503
@@ -781,14 +785,14 @@ def test_sync_client_retries_500_then_raises(self, mock_sleep: Any) -> None:
781785
)
782786
retry_transport = _RetryTransport(
783787
transport=fake, # type: ignore[arg-type]
784-
retry_config=RetryConfig(max_retries=3, backoff_factor=2.0, max_wait=0.01),
788+
# max_retries=2 → 3 total attempts (initial + 2 retries)
789+
retry_config=RetryConfig(max_retries=2, backoff_factor=2.0, max_wait=0.01),
785790
)
786791
client._client._transport = retry_transport # type: ignore[assignment]
787792

788793
with pytest.raises(ApiError) as exc_info:
789794
client.get("/indexes")
790795
assert exc_info.value.status_code == 500
791-
# max_retries=3 total
792796
assert fake.call_count == 3
793797

794798
@patch("pinecone._internal.http_client.time.sleep")

tests/unit/test_retry_config.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ def test_pinecone_config_custom_retry_config(self) -> None:
104104
class TestCustomRetryConfigSync:
105105
@patch("pinecone._internal.http_client.time.sleep")
106106
def test_custom_retry_config_max_retries(self, mock_sleep: Any) -> None:
107-
"""Custom RetryConfig with max_retries=2 only retries once."""
107+
"""Custom RetryConfig with max_retries=1 only retries once (2 total attempts)."""
108108
fake = _FakeTransport(
109109
[
110110
httpx.Response(500, json={"message": "error"}),
@@ -114,7 +114,8 @@ def test_custom_retry_config_max_retries(self, mock_sleep: Any) -> None:
114114
)
115115
transport = _RetryTransport(
116116
transport=fake, # type: ignore[arg-type]
117-
retry_config=RetryConfig(max_retries=2, max_wait=0.01),
117+
# max_retries=1 → 2 total attempts (initial + 1 retry)
118+
retry_config=RetryConfig(max_retries=1, max_wait=0.01),
118119
)
119120
response = transport.handle_request(_get_request())
120121
assert response.status_code == 500
@@ -203,7 +204,7 @@ class TestCustomRetryConfigAsync:
203204
@patch("pinecone._internal.http_client.asyncio.sleep")
204205
@pytest.mark.asyncio
205206
async def test_custom_retry_config_max_retries(self, mock_sleep: Any) -> None:
206-
"""Custom RetryConfig with max_retries=2 only retries once."""
207+
"""Custom RetryConfig with max_retries=1 only retries once (2 total attempts)."""
207208
fake = _FakeAsyncTransport(
208209
[
209210
httpx.Response(500, json={"message": "error"}),
@@ -213,7 +214,8 @@ async def test_custom_retry_config_max_retries(self, mock_sleep: Any) -> None:
213214
)
214215
transport = _AsyncRetryTransport(
215216
transport=fake, # type: ignore[arg-type]
216-
retry_config=RetryConfig(max_retries=2, max_wait=0.01),
217+
# max_retries=1 → 2 total attempts (initial + 1 retry)
218+
retry_config=RetryConfig(max_retries=1, max_wait=0.01),
217219
)
218220
response = await transport.handle_async_request(_get_request())
219221
assert response.status_code == 500

0 commit comments

Comments
 (0)