fix(forgejo-package-prune): correct version enumeration + network robustness
Forgejo has no per-package /versions endpoint — the list endpoint returns one entry per (name, version). build_delete_plans now groups list results by name instead of calling a 404ing /versions sub-path. Removed dead list_versions/_paginate. Added retry + longer timeout to _api_request for slow/large registry listings (container list was timing out). Updated fixture test to the grouped model. Dry-run now clean: 29 candidate deletions across 5 container packages, 0 errors (was 53× HTTP 404). ACTIVITY-WP-0020 T02 fix; enable/apply still gated on protection-coverage review (T05). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
20bf3d5942
commit
6f7ca31a6d
2 changed files with 43 additions and 73 deletions
|
|
@ -8,6 +8,7 @@ import json
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
import sys
|
import sys
|
||||||
|
import time
|
||||||
import urllib.error
|
import urllib.error
|
||||||
import urllib.parse
|
import urllib.parse
|
||||||
import urllib.request
|
import urllib.request
|
||||||
|
|
@ -111,7 +112,8 @@ def _api_request(
|
||||||
url: str,
|
url: str,
|
||||||
token: str,
|
token: str,
|
||||||
*,
|
*,
|
||||||
timeout: float = 30.0,
|
timeout: float = 60.0,
|
||||||
|
retries: int = 2,
|
||||||
) -> Any:
|
) -> Any:
|
||||||
req = urllib.request.Request(
|
req = urllib.request.Request(
|
||||||
url,
|
url,
|
||||||
|
|
@ -121,37 +123,23 @@ def _api_request(
|
||||||
"Accept": "application/json",
|
"Accept": "application/json",
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
last_exc: Exception | None = None
|
||||||
|
for attempt in range(retries + 1):
|
||||||
|
try:
|
||||||
with urllib.request.urlopen(req, timeout=timeout) as resp:
|
with urllib.request.urlopen(req, timeout=timeout) as resp:
|
||||||
body = resp.read().decode("utf-8")
|
body = resp.read().decode("utf-8")
|
||||||
return json.loads(body) if body else None
|
return json.loads(body) if body else None
|
||||||
|
except urllib.error.HTTPError:
|
||||||
|
raise # 4xx/5xx are real responses — surface them, do not retry
|
||||||
def _paginate(
|
except (urllib.error.URLError, TimeoutError, OSError) as exc:
|
||||||
base_url: str,
|
# Transient: connection reset, read timeout, TLS handshake timeout.
|
||||||
token: str,
|
last_exc = exc
|
||||||
path: str,
|
if attempt < retries:
|
||||||
*,
|
time.sleep(2 * (attempt + 1))
|
||||||
page_size: int = 50,
|
continue
|
||||||
) -> list[dict[str, Any]]:
|
raise
|
||||||
items: list[dict[str, Any]] = []
|
if last_exc: # pragma: no cover - defensive
|
||||||
page = 1
|
raise last_exc
|
||||||
while True:
|
|
||||||
query = urllib.parse.urlencode({"limit": page_size, "page": page})
|
|
||||||
url = f"{base_url.rstrip('/')}{path}?{query}"
|
|
||||||
payload = _api_request("GET", url, token)
|
|
||||||
if not payload:
|
|
||||||
break
|
|
||||||
if isinstance(payload, list):
|
|
||||||
batch = payload
|
|
||||||
else:
|
|
||||||
batch = payload.get("data") or payload.get("items") or []
|
|
||||||
if not batch:
|
|
||||||
break
|
|
||||||
items.extend(batch)
|
|
||||||
if len(batch) < page_size:
|
|
||||||
break
|
|
||||||
page += 1
|
|
||||||
return items
|
|
||||||
|
|
||||||
|
|
||||||
def list_packages(
|
def list_packages(
|
||||||
|
|
@ -180,23 +168,6 @@ def list_packages(
|
||||||
return items
|
return items
|
||||||
|
|
||||||
|
|
||||||
def list_versions(
|
|
||||||
base_url: str,
|
|
||||||
token: str,
|
|
||||||
owner: str,
|
|
||||||
package_type: str,
|
|
||||||
name: str,
|
|
||||||
) -> list[dict[str, Any]]:
|
|
||||||
owner_q = urllib.parse.quote(owner)
|
|
||||||
type_q = urllib.parse.quote(package_type)
|
|
||||||
name_q = urllib.parse.quote(name, safe="")
|
|
||||||
return _paginate(
|
|
||||||
base_url,
|
|
||||||
token,
|
|
||||||
f"/api/v1/packages/{owner_q}/{type_q}/{name_q}/versions",
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def delete_version(
|
def delete_version(
|
||||||
base_url: str,
|
base_url: str,
|
||||||
token: str,
|
token: str,
|
||||||
|
|
@ -240,19 +211,17 @@ def build_delete_plans(
|
||||||
errors.append(f"list {package_type}: {exc}")
|
errors.append(f"list {package_type}: {exc}")
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
# Forgejo's package list endpoint returns one entry per (name, version).
|
||||||
|
# Group by package name; each group is that package's version set — there
|
||||||
|
# is no separate per-package "/versions" endpoint.
|
||||||
|
by_name: dict[str, list[dict[str, Any]]] = {}
|
||||||
for package in packages:
|
for package in packages:
|
||||||
name = str(package.get("name") or package.get("package_name") or "")
|
name = str(package.get("name") or package.get("package_name") or "")
|
||||||
if not name:
|
if not name:
|
||||||
continue
|
continue
|
||||||
try:
|
by_name.setdefault(name, []).append(package)
|
||||||
versions = list_versions(base_url, token, owner, package_type, name)
|
|
||||||
except urllib.error.HTTPError as exc:
|
|
||||||
errors.append(f"versions {package_type}/{name}: HTTP {exc.code}")
|
|
||||||
continue
|
|
||||||
except Exception as exc: # noqa: BLE001
|
|
||||||
errors.append(f"versions {package_type}/{name}: {exc}")
|
|
||||||
continue
|
|
||||||
|
|
||||||
|
for name, versions in by_name.items():
|
||||||
sorted_versions = sorted(
|
sorted_versions = sorted(
|
||||||
versions,
|
versions,
|
||||||
key=lambda item: _parse_created_at(str(item.get("created_at") or "")),
|
key=lambda item: _parse_created_at(str(item.get("created_at") or "")),
|
||||||
|
|
|
||||||
|
|
@ -37,16 +37,17 @@ image:
|
||||||
|
|
||||||
def test_build_delete_plans_respects_depth_and_protection(self) -> None:
|
def test_build_delete_plans_respects_depth_and_protection(self) -> None:
|
||||||
protected = {("container", "vergabe-teilnahme", "old-prod")}
|
protected = {("container", "vergabe-teilnahme", "old-prod")}
|
||||||
versions = [
|
# Forgejo's package list endpoint returns one entry per (name, version);
|
||||||
{"version": "v4", "created_at": "2026-07-12T10:00:00Z"},
|
# build_delete_plans groups these by name (no separate /versions call).
|
||||||
{"version": "v3", "created_at": "2026-07-11T10:00:00Z"},
|
packages = [
|
||||||
{"version": "v2", "created_at": "2026-07-10T10:00:00Z"},
|
{"name": "vergabe-teilnahme", "version": "v4", "created_at": "2026-07-12T10:00:00Z"},
|
||||||
{"version": "v1", "created_at": "2026-07-09T10:00:00Z"},
|
{"name": "vergabe-teilnahme", "version": "v3", "created_at": "2026-07-11T10:00:00Z"},
|
||||||
{"version": "old-prod", "created_at": "2026-07-08T10:00:00Z"},
|
{"name": "vergabe-teilnahme", "version": "v2", "created_at": "2026-07-10T10:00:00Z"},
|
||||||
|
{"name": "vergabe-teilnahme", "version": "v1", "created_at": "2026-07-09T10:00:00Z"},
|
||||||
|
{"name": "vergabe-teilnahme", "version": "old-prod", "created_at": "2026-07-08T10:00:00Z"},
|
||||||
]
|
]
|
||||||
|
|
||||||
with mock.patch.object(prune, "list_packages", return_value=[{"name": "vergabe-teilnahme"}]):
|
with mock.patch.object(prune, "list_packages", return_value=packages):
|
||||||
with mock.patch.object(prune, "list_versions", return_value=versions):
|
|
||||||
plans, errors = prune.build_delete_plans(
|
plans, errors = prune.build_delete_plans(
|
||||||
base_url="https://forgejo.example",
|
base_url="https://forgejo.example",
|
||||||
token="token",
|
token="token",
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue