Skip to content

Commit fcea2d1

Browse files
committed
fix autopagination loop on non paginated api endpoint
1 parent d0492b7 commit fcea2d1

3 files changed

Lines changed: 172 additions & 8 deletions

File tree

src/pytfe/resources/_base.py

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from typing import Any
88

99
from .._http import HTTPTransport
10+
from .._logging import logger
1011

1112

1213
def _to_int(value: Any) -> int | None:
@@ -26,9 +27,26 @@ def __init__(self, t: HTTPTransport) -> None:
2627
self.t = t
2728

2829
def _list(
29-
self, path: str, *, params: dict | None = None
30+
self, path: str, *, params: dict | None = None, paginated: bool = True
3031
) -> Iterator[dict[str, Any]]:
3132
base_params = dict(params or {})
33+
34+
# Some TFE endpoints are not paginated: they return the full collection
35+
# in a single response, ignore page[number]/page[size], and emit no
36+
# meta.pagination block (e.g. workspace /vars and /all-vars). Callers
37+
# opt those out so we issue exactly one request and never loop trying to
38+
# fetch a "next" page that re-returns the same set
39+
# (see hashicorp/python-tfe#181).
40+
if not paginated:
41+
r = self.t.request("GET", path, params=base_params)
42+
json_response = r.json()
43+
if not isinstance(json_response, dict):
44+
json_response = {}
45+
data = json_response.get("data", [])
46+
if isinstance(data, list):
47+
yield from data
48+
return
49+
3250
page = int(base_params.get("page[number]", 1))
3351
while True:
3452
p = dict(base_params)
@@ -81,8 +99,18 @@ def _list(
8199
# Metadata present and indicates no next page.
82100
break
83101

84-
# Fallback for endpoints that do not return pagination metadata.
85-
page_size = int(p["page[size]"])
86-
if len(data) < page_size:
87-
break
88-
page += 1
102+
# No pagination metadata. Genuine TFE list endpoints always include
103+
# meta.pagination, so a response without it is a non-paginated
104+
# collection returned in full. Treat it as a single complete page
105+
# and stop. Re-requesting would loop forever when the endpoint
106+
# ignores page[number]/page[size] and re-returns the same full set
107+
# (the root cause of hashicorp/python-tfe#181).
108+
if len(data) >= int(p["page[size]"]):
109+
logger.debug(
110+
"List endpoint %s returned a full page (%d rows) without "
111+
"pagination metadata; treating it as a single, "
112+
"non-paginated response.",
113+
path,
114+
len(data),
115+
)
116+
break

src/pytfe/resources/variable.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,16 @@ def list(
3030
if not valid_string_id(workspace_id):
3131
raise ValueError(ERR_INVALID_WORKSPACE_ID)
3232

33+
# The /vars endpoint is not paginated: it returns every variable in a
34+
# single response and ignores page[number]/page[size]. Opt out of the
35+
# pagination loop so we issue exactly one request (python-tfe#181).
3336
path = f"/api/v2/workspaces/{workspace_id}/vars"
3437
params: dict[str, Any] = {}
3538
if options:
3639
# Add any options if needed in the future
3740
pass
3841

39-
for item in self._list(path, params=params):
42+
for item in self._list(path, params=params, paginated=False):
4043
attr = item.get("attributes", {}) or {}
4144
var_id = item.get("id", "")
4245
variable_data = dict(attr)
@@ -50,13 +53,14 @@ def list_all(
5053
if not valid_string_id(workspace_id):
5154
raise ValueError(ERR_INVALID_WORKSPACE_ID)
5255

56+
# Like /vars, the /all-vars endpoint is not paginated; request once.
5357
path = f"/api/v2/workspaces/{workspace_id}/all-vars"
5458
params: dict[str, Any] = {}
5559
if options:
5660
# Add any options if needed in the future
5761
pass
5862

59-
for item in self._list(path, params=params):
63+
for item in self._list(path, params=params, paginated=False):
6064
attr = item.get("attributes", {}) or {}
6165
var_id = item.get("id", "")
6266
variable_data = dict(attr)

tests/units/test_variable.py

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
# Copyright IBM Corp. 2025, 2026
2+
# SPDX-License-Identifier: MPL-2.0
3+
4+
"""Unit tests for the workspace Variables resource.
5+
6+
The headline cases here are regression tests for hashicorp/python-tfe#181:
7+
``variables.list()`` infinite-looping on workspaces with >= 100 variables
8+
because the ``/vars`` (and ``/all-vars``) endpoints are not paginated.
9+
"""
10+
11+
from unittest.mock import Mock
12+
13+
import pytest
14+
15+
from pytfe._http import HTTPTransport
16+
from pytfe.errors import ERR_INVALID_WORKSPACE_ID
17+
from pytfe.models.variable import Variable
18+
from pytfe.resources._base import _Service
19+
from pytfe.resources.variable import Variables
20+
21+
22+
def _vars_payload(count: int) -> dict:
23+
"""A /vars-style response: full set in one page, no meta.pagination."""
24+
return {
25+
"data": [
26+
{
27+
"id": f"var-{i}",
28+
"type": "vars",
29+
"attributes": {"key": f"key-{i}", "value": f"value-{i}"},
30+
}
31+
for i in range(count)
32+
]
33+
}
34+
35+
36+
class TestVariablesList:
37+
"""Tests for Variables.list / list_all."""
38+
39+
def setup_method(self):
40+
self.mock_transport = Mock(spec=HTTPTransport)
41+
self.variables = Variables(self.mock_transport)
42+
self.workspace_id = "ws-test123"
43+
44+
def test_list_validations(self):
45+
with pytest.raises(ValueError, match=ERR_INVALID_WORKSPACE_ID):
46+
list(self.variables.list(""))
47+
with pytest.raises(ValueError, match=ERR_INVALID_WORKSPACE_ID):
48+
list(self.variables.list(None))
49+
50+
def test_list_all_validations(self):
51+
with pytest.raises(ValueError, match=ERR_INVALID_WORKSPACE_ID):
52+
list(self.variables.list_all(""))
53+
with pytest.raises(ValueError, match=ERR_INVALID_WORKSPACE_ID):
54+
list(self.variables.list_all(None))
55+
56+
def test_list_does_not_paginate_with_100_plus_variables(self):
57+
"""Regression for #181: a workspace with >= 100 vars must not loop.
58+
59+
The endpoint ignores page params and re-returns the full set, so the
60+
old pagination heuristic looped forever. We now issue exactly one
61+
request and return each variable once.
62+
"""
63+
response = Mock()
64+
response.json.return_value = _vars_payload(150)
65+
self.mock_transport.request.return_value = response
66+
67+
result = list(self.variables.list(self.workspace_id))
68+
69+
# Exactly one request — no follow-up page fetches.
70+
self.mock_transport.request.assert_called_once_with(
71+
"GET",
72+
f"/api/v2/workspaces/{self.workspace_id}/vars",
73+
params={},
74+
)
75+
# All 150 variables, no duplication.
76+
assert len(result) == 150
77+
assert all(isinstance(v, Variable) for v in result)
78+
assert [v.id for v in result] == [f"var-{i}" for i in range(150)]
79+
80+
def test_list_all_does_not_paginate_with_100_plus_variables(self):
81+
response = Mock()
82+
response.json.return_value = _vars_payload(120)
83+
self.mock_transport.request.return_value = response
84+
85+
result = list(self.variables.list_all(self.workspace_id))
86+
87+
self.mock_transport.request.assert_called_once_with(
88+
"GET",
89+
f"/api/v2/workspaces/{self.workspace_id}/all-vars",
90+
params={},
91+
)
92+
assert len(result) == 120
93+
assert [v.id for v in result] == [f"var-{i}" for i in range(120)]
94+
95+
def test_list_exactly_100_variables(self):
96+
"""The exactly-page-size boundary also looped under the old logic."""
97+
response = Mock()
98+
response.json.return_value = _vars_payload(100)
99+
self.mock_transport.request.return_value = response
100+
101+
result = list(self.variables.list(self.workspace_id))
102+
103+
self.mock_transport.request.assert_called_once()
104+
assert len(result) == 100
105+
106+
def test_list_empty(self):
107+
response = Mock()
108+
response.json.return_value = {"data": []}
109+
self.mock_transport.request.return_value = response
110+
111+
result = list(self.variables.list(self.workspace_id))
112+
113+
self.mock_transport.request.assert_called_once()
114+
assert result == []
115+
116+
117+
class TestListSafetyNet:
118+
"""The generic _list safety net protects any non-paginated endpoint."""
119+
120+
def test_full_page_without_metadata_is_treated_as_single_page(self):
121+
"""A paginated (paginated=True) call that gets a full page with no
122+
meta.pagination must stop after one request rather than loop."""
123+
transport = Mock(spec=HTTPTransport)
124+
response = Mock()
125+
response.json.return_value = _vars_payload(100) # full page, no meta
126+
transport.request.return_value = response
127+
128+
service = _Service(transport)
129+
result = list(service._list("/api/v2/some/unpaginated", params={}))
130+
131+
transport.request.assert_called_once()
132+
assert len(result) == 100

0 commit comments

Comments
 (0)