fix autopagination loop on non paginated api endpoint (#182)

Author: iam404

AGENTS.md

@@ -73,6 +73,7 @@ These are mistakes a competent Python developer would make if they hadn't read t
 - **Don't catch `httpx` errors directly.** The transport already translates them into `TFEError` subclasses. Catching `httpx.HTTPError` in a resource means the typed error never propagates.
 - **Always send the bearer token, even to absolute URLs returned by the API.** Endpoints like `hosted_state_download_url`, `hosted_state_upload_url`, plan `json-output`, and apply `errored-state` redirect to `archivist.terraform.io` — which is HashiCorp infrastructure that *requires* the bearer. go-tfe does the same (see `state_version.go::Download` + `tfe.go::NewRequest`). Stripping the bearer breaks downstream consumers (notably the Ansible collection's statefile + dynamic-inventory flows). `HTTPTransport.request` accepts `include_auth=False` only as an opt-out for the hypothetical case of calling a genuinely non-HashiCorp host; do not use it for Archivist URLs.
 - **Don't write a custom page loop.** `self._list(path, params=...)` handles pagination + non-paginated endpoints transparently. Rolling your own loop will diverge from the rest of the codebase.
+- **Disable pagination for endpoints that ignore page params.** A few endpoints return the *whole* collection on every request and ignore `page[number]`/`page[size]` (workspace `/vars` and `/all-vars`). Call `self._list(path, params=..., paginated=False)` for those — otherwise they infinite-loop once the collection reaches the page size (the [#181](https://github.com/hashicorp/python-tfe/issues/181) bug). See [ITERATORS.md](docs/ITERATORS.md).
 - **Don't reuse generators.** Iterators returned by `list_*` are single-use. If you need to traverse twice, `materialized = list(client.foo.list_bars(...))` first.
 - **Don't add features beyond what was asked.** This codebase is approaching v1.0.0. Adding "while I'm here" refactors or speculative abstractions slows reviews and risks breaking the Ansible collection.
 - **Don't assume every successful response is `{"data": ...}`.** Check the docs/go-tfe/spec for each endpoint: some return a JSON:API envelope, some return a bare resource object, `204 No Content`, `null`, raw bytes, or a redirect to a blob URL. Add tests for non-standard shapes.

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Unreleased
 
+## Bug Fixes
+
+### Pagination
+* Fixed `list_*` infinite-looping for API call which are non paginated, so they are now fetched with a single request. The generic list helper also treats any response without `meta.pagination` as a single complete page, preventing the same loop on other non-paginated endpoints. [#181](https://github.com/hashicorp/python-tfe/issues/181)
+
 
 # Released
 # v1.0.0

docs/ITERATORS.md

@@ -56,7 +56,23 @@ def list(
         yield self._workspace_from(item)
 ```
 
-That's it. `self._list()` lives in `_base.py` and handles `page[number]` / `page[size]` and follow-through automatically. It is also robust to endpoints that **don't paginate** — if the response has no pagination metadata and the returned data is smaller than the requested page size, the helper just breaks after one round-trip. So you do not need a different code path for relationship reads like `GET /workspaces/{id}/tag-bindings` (single response) versus list endpoints like `GET /organizations/{org}/workspaces` (paginated). The same `for item in self._list(path): yield ...` works for both.
+That's it. `self._list()` lives in `_base.py` and handles `page[number]` / `page[size]` and follow-through automatically. When a response carries no `meta.pagination` block, `_list` treats it as a single complete page and stops after one round-trip — so the same `for item in self._list(path): yield ...` works for ordinary paginated endpoints (`GET /organizations/{org}/workspaces`) and for single-response relationship reads (`GET /workspaces/{id}/tag-bindings`) alike.
+
+### Endpoints that ignore pagination entirely — pass `paginated=False`
+
+A few HCP Terraform endpoints return the **whole** collection on every request and ignore `page[number]` / `page[size]`. The workspace **`/vars`** and **`/all-vars`** endpoints are the known ones. For these you must opt out of the page loop explicitly:
+
+```python
+# variable.py — /vars is not paginated
+for item in self._list(path, params=params, paginated=False):
+    yield self._variable_from(item)
+```
+
+With `paginated=False`, `_list` issues exactly one request and yields every row.
+
+Why this matters: skipping the flag on such an endpoint with **≥ `page_size` rows (default 100)** used to spin forever — the helper saw a "full" page, asked for page 2, got the *same* full set back (the endpoint ignored the page param), and re-yielded it, indefinitely. That was the root cause of [#181](https://github.com/hashicorp/python-tfe/issues/181). The generic "no `meta.pagination` ⇒ single page" rule now catches this as a safety net, but **still set `paginated=False`** on a known non-paginated endpoint: it documents intent and avoids sending meaningless page params.
+
+> Rule of thumb: if the endpoint returns no `meta.pagination` and ignores `page[size]` (check go-tfe or the API docs — it returns the full collection in one shot), pass `paginated=False`.
 
 
 ### Note on lazy validation
@@ -131,7 +147,7 @@ def list_versions(
 
 `registry_module.list_versions` is the only method in the codebase that does this. Add a docstring note explaining the reason if you find yourself reaching for this pattern, so future readers don't mistake it for something to copy.
 
-Do **not** reach for `iter(list)` just because the endpoint is non-paginated. Use `self._list()` for those — that's the convention.
+Do **not** reach for `iter(list)` just because the endpoint is non-paginated. Use `self._list()` for those — that's the convention (with `paginated=False` if the endpoint ignores `page[size]` and returns the full set, as described above).
 
 ### Shape that does **not** match the convention (don't do this)
 
@@ -186,6 +202,7 @@ Don't assert `isinstance(result, list)` against the raw return — that asserts
 - [ ] Every `list*` method returns `Iterator[X]`, not `list[X]` or `Iterable[X]`
 - [ ] `Iterator` is imported from `collections.abc`, not `typing`
 - [ ] The body uses the canonical `for item in self._list(path, params=params): yield ...` pattern — including for non-paginated single-shot endpoints
+- [ ] Endpoints that ignore `page[size]` and return the whole collection (e.g. workspace `/vars`, `/all-vars`) pass `paginated=False` to `self._list(...)` — otherwise they infinite-loop at ≥ 100 rows (see [#181](https://github.com/hashicorp/python-tfe/issues/181))
 - [ ] Hand-rolled `iter(materialized_list)` only appears if the method has a try/except fallback to a different endpoint (extremely rare — has a docstring note explaining why)
 - [ ] If a class defines `def list(...)`, later annotations in that class avoid bare `list[...]` so mypy does not resolve `list` to the method
 - [ ] Examples that call the method use `list(client.foo.list_bars(...))` (or stream with a `for` loop) — never assume list semantics on the bare return

docs/MODELS.md

@@ -59,7 +59,7 @@ class Run(BaseModel):
 Rules:
 
 - **Every multi-word JSON:API attribute** gets an alias. Don't try to invent a snake_case-to-hyphen mapper — be explicit per field.
-- **Page params** use the JSON:API square-bracket form: `Field(None, alias="page[number]")`, `Field(None, alias="page[size]")`.
+- **Page params** use the JSON:API square-bracket form: `Field(None, alias="page[number]")`, `Field(None, alias="page[size]")`. Note that a few endpoints (workspace `/vars`, `/all-vars`) are not paginated and ignore these — their resource methods call `self._list(..., paginated=False)`, so a `page_size` field on those options models would be a no-op. See [ITERATORS.md](ITERATORS.md).
 - **Filter params** use the same convention: `Field(None, alias="filter[workspace][name]")`.
 - **`include`** is a comma-separated string on the wire but exposed as `list[SomeEnum] | None` in Python; the resource layer dumps options with `mode="json"` and joins the resulting values (`",".join(params["include"])`). See the `policy_set.read_with_options` pattern.
 

docs/pagination.md

@@ -82,8 +82,12 @@ for run in client.runs.list("ws-abc123", options):
 - `list` / `list_*` methods are lazy. If an invalid-id check is inside a
   generator method, the exception is raised when you iterate, not when you
   create the iterator.
-- Some relationship endpoints are not paginated by the server, but pyTFE still
-  exposes them as iterators for a consistent public API.
+- Some endpoints are not paginated by the server, but pyTFE still exposes them
+  as iterators for a consistent public API. A few — notably
+  `variables.list()` / `variables.list_all()` (the workspace `/vars` and
+  `/all-vars` endpoints) — return the entire collection in a single request and
+  ignore `page_size`. You still iterate them the same way; the SDK just fetches
+  everything in one round-trip.
 - A small number of older methods intentionally return concrete lists for
   backward compatibility. Prefer the iterator rule for new code, and check the
   method's return type if you are unsure.

src/pytfe/resources/_base.py

@@ -7,6 +7,7 @@
 from typing import Any
 
 from .._http import HTTPTransport
+from .._logging import logger
 
 
 def _to_int(value: Any) -> int | None:
@@ -26,9 +27,25 @@ def __init__(self, t: HTTPTransport) -> None:
         self.t = t
 
     def _list(
-        self, path: str, *, params: dict | None = None
+        self, path: str, *, params: dict | None = None, paginated: bool = True
     ) -> Iterator[dict[str, Any]]:
         base_params = dict(params or {})
+
+        # Some TFE endpoints are not paginated: they return the full collection
+        # in a single response, ignore page[number]/page[size], and emit no
+        # meta.pagination block (e.g. workspace /vars and /all-vars). Callers
+        # opt those out so we issue exactly one request and never loop trying to
+        # fetch a "next" page that re-returns the same set
+        if not paginated:
+            r = self.t.request("GET", path, params=base_params)
+            json_response = r.json()
+            if not isinstance(json_response, dict):
+                json_response = {}
+            data = json_response.get("data", [])
+            if isinstance(data, list):
+                yield from data
+            return
+
         page = int(base_params.get("page[number]", 1))
         while True:
             p = dict(base_params)
@@ -81,8 +98,17 @@ def _list(
                 # Metadata present and indicates no next page.
                 break
 
-            # Fallback for endpoints that do not return pagination metadata.
-            page_size = int(p["page[size]"])
-            if len(data) < page_size:
-                break
-            page += 1
+            # No pagination metadata. Genuine TFE list endpoints always include
+            # meta.pagination, so a response without it is a non-paginated
+            # collection returned in full. Treat it as a single complete page
+            # and stop. Re-requesting would loop forever when the endpoint
+            # ignores page[number]/page[size] and re-returns the same full set
+            if len(data) >= int(p["page[size]"]):
+                logger.debug(
+                    "List endpoint %s returned a full page (%d rows) without "
+                    "pagination metadata; treating it as a single, "
+                    "non-paginated response.",
+                    path,
+                    len(data),
+                )
+            break

src/pytfe/resources/no_code_module.py

@@ -255,11 +255,12 @@ def read_variables(
         if not valid_string(version):
             raise InvalidVersionError()
 
+        # module-variables is not paginated; fetch the full set in one request.
         path = (
             f"/api/v2/no-code-modules/{no_code_module_id}"
             f"/versions/{version}/module-variables"
         )
-        for item in self._list(path):
+        for item in self._list(path, paginated=False):
             attrs = item.get("attributes") or {}
             yield RegistryModuleVariable.model_validate(
                 {

src/pytfe/resources/projects.py

@@ -320,8 +320,9 @@ def list_effective_tag_bindings(
         if not valid_string_id(project_id):
             raise ValueError("Project ID is required and must be valid")
 
+        # effective-tag-bindings is not paginated.
         path = f"/api/v2/projects/{project_id}/effective-tag-bindings"
-        for item in self._list(path):
+        for item in self._list(path, paginated=False):
             attr = item.get("attributes", {}) or {}
             links = item.get("links", {}) or {}
             yield EffectiveTagBinding(

src/pytfe/resources/run_event.py

@@ -26,8 +26,9 @@ def list(
         params: dict[str, Any] = {}
         if options and options.include:
             params["include"] = ",".join(options.include)
+        # The run-events endpoint is not paginated; fetch the full set in one request.
         path = f"/api/v2/runs/{run_id}/run-events"
-        for item in self._list(path, params=params):
+        for item in self._list(path, params=params, paginated=False):
             attrs = item.get("attributes", {})
             attrs["id"] = item.get("id")
             yield RunEvent.model_validate(attrs)

src/pytfe/resources/task_stage.py

@@ -76,9 +76,9 @@ def list(
             raise InvalidRunIDError()
 
         path = f"/api/v2/runs/{run_id}/task-stages"
-        kwargs = {"params": options.model_dump(by_alias=True)} if options else {}
+        params = options.model_dump(by_alias=True) if options else None
 
-        for item in self._list(path, **kwargs):
+        for item in self._list(path, params=params):
             yield self._parse_task_stage(item)
 
     # Override