refactor(nix): split NixFlakeInstaller into atomic modules and add GitHub 403 retry handling
Some checks failed
Mark stable commit / test-unit (push) Has been cancelled
Mark stable commit / test-integration (push) Has been cancelled
Mark stable commit / test-env-virtual (push) Has been cancelled
Mark stable commit / test-env-nix (push) Has been cancelled
Mark stable commit / test-e2e (push) Has been cancelled
Mark stable commit / test-virgin-user (push) Has been cancelled
Mark stable commit / test-virgin-root (push) Has been cancelled
Mark stable commit / codesniffer-shellcheck (push) Has been cancelled
Mark stable commit / codesniffer-ruff (push) Has been cancelled
Mark stable commit / mark-stable (push) Has been cancelled

- Move Nix flake installer into installers/nix/ with atomic components
  (installer, runner, profile, retry, types)
- Preserve legacy behavior and semantics of NixFlakeInstaller
- Add GitHub API 403 rate-limit retry with Fibonacci backoff + jitter
- Update all imports to new nix module path
- Rename legacy unit tests and adapt patches to new structure
- Add unit test for simulated GitHub 403 retry without realtime sleeping

https://chatgpt.com/share/693e925d-a79c-800f-b0b6-92b8ba260b11
This commit is contained in:
Kevin Veen-Birkenbach
2025-12-14 11:32:48 +01:00
parent 3990560cd7
commit d55c8d3726
13 changed files with 524 additions and 263 deletions

View File

@@ -22,7 +22,7 @@ import unittest
from contextlib import redirect_stdout
from unittest.mock import patch
from pkgmgr.actions.install.installers.nix_flake import NixFlakeInstaller
from pkgmgr.actions.install.installers.nix import NixFlakeInstaller
class DummyCtx:
@@ -66,7 +66,7 @@ class TestNixFlakeInstaller(unittest.TestCase):
@staticmethod
def _enable_nix_in_module(which_patch) -> None:
"""Ensure shutil.which('nix') in nix_flake module returns a path."""
"""Ensure shutil.which('nix') in nix module returns a path."""
which_patch.return_value = "/usr/bin/nix"
def test_nix_flake_run_success(self) -> None:
@@ -78,14 +78,14 @@ class TestNixFlakeInstaller(unittest.TestCase):
installer = NixFlakeInstaller()
buf = io.StringIO()
with patch("pkgmgr.actions.install.installers.nix_flake.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix_flake.subprocess.run"
) as subproc_mock, patch(
"pkgmgr.actions.install.installers.nix_flake.run_command"
) as run_cmd_mock, redirect_stdout(buf):
with patch("pkgmgr.actions.install.installers.nix.installer.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix.installer.os.path.exists", return_value=True
), patch(
"pkgmgr.actions.install.installers.nix.runner.subprocess.run"
) as subproc_mock, redirect_stdout(buf):
self._enable_nix_in_module(which_mock)
# For profile list JSON (used only on failure paths, but keep deterministic)
subproc_mock.return_value = subprocess.CompletedProcess(
args=["nix", "profile", "list", "--json"],
returncode=0,
@@ -119,14 +119,14 @@ class TestNixFlakeInstaller(unittest.TestCase):
installer = NixFlakeInstaller()
buf = io.StringIO()
with patch("pkgmgr.actions.install.installers.nix_flake.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix_flake.subprocess.run"
) as subproc_mock, patch(
"pkgmgr.actions.install.installers.nix_flake.run_command"
) as run_cmd_mock, redirect_stdout(buf):
with patch("pkgmgr.actions.install.installers.nix.installer.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix.installer.os.path.exists", return_value=True
), patch(
"pkgmgr.actions.install.installers.nix.runner.subprocess.run"
) as subproc_mock, redirect_stdout(buf):
self._enable_nix_in_module(which_mock)
# No indices available (empty list)
subproc_mock.return_value = subprocess.CompletedProcess(
args=["nix", "profile", "list", "--json"],
returncode=0,
@@ -157,14 +157,14 @@ class TestNixFlakeInstaller(unittest.TestCase):
installer = NixFlakeInstaller()
buf = io.StringIO()
with patch("pkgmgr.actions.install.installers.nix_flake.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix_flake.subprocess.run"
) as subproc_mock, patch(
"pkgmgr.actions.install.installers.nix_flake.run_command"
) as run_cmd_mock, redirect_stdout(buf):
with patch("pkgmgr.actions.install.installers.nix.installer.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix.installer.os.path.exists", return_value=True
), patch(
"pkgmgr.actions.install.installers.nix.runner.subprocess.run"
) as subproc_mock, redirect_stdout(buf):
self._enable_nix_in_module(which_mock)
# No indices available (empty list)
subproc_mock.return_value = subprocess.CompletedProcess(
args=["nix", "profile", "list", "--json"],
returncode=0,
@@ -209,11 +209,12 @@ class TestNixFlakeInstaller(unittest.TestCase):
ctx = DummyCtx(identifier="pkgmgr", repo_dir=self.repo_dir, quiet=False)
installer = NixFlakeInstaller()
with patch("pkgmgr.actions.install.installers.nix_flake.shutil.which") as which_mock:
with patch("pkgmgr.actions.install.installers.nix.installer.shutil.which") as which_mock, patch(
"pkgmgr.actions.install.installers.nix.installer.os.path.exists", return_value=True
):
self._enable_nix_in_module(which_mock)
os.environ["PKGMGR_DISABLE_NIX_FLAKE_INSTALLER"] = "1"
self.assertFalse(installer.supports(ctx))
if __name__ == "__main__":
unittest.main()

View File

@@ -0,0 +1,106 @@
from __future__ import annotations
import unittest
from unittest.mock import patch
from pkgmgr.actions.install.installers.nix.retry import GitHubRateLimitRetry, RetryPolicy
from pkgmgr.actions.install.installers.nix.types import RunResult
class DummyCtx:
def __init__(self, quiet: bool = True) -> None:
self.quiet = quiet
class FakeRunner:
"""
Simulates a runner that returns:
- HTTP 403 for the first N calls
- success afterwards
"""
def __init__(self, fail_count: int) -> None:
self.fail_count = fail_count
self.calls = 0
def run(self, ctx: DummyCtx, cmd: str, allow_failure: bool) -> RunResult:
self.calls += 1
if self.calls <= self.fail_count:
return RunResult(
returncode=1,
stdout="",
stderr="error: HTTP error 403: rate limit exceeded (simulated)",
)
return RunResult(returncode=0, stdout="ok", stderr="")
class TestGitHub403Retry(unittest.TestCase):
def test_retries_on_403_without_realtime_waiting(self) -> None:
"""
Ensure:
- It retries only on GitHub 403-like errors
- It does not actually sleep in realtime (time.sleep patched)
- It stops once a success occurs
- Wait times follow Fibonacci(base=30) + jitter
"""
policy = RetryPolicy(
max_attempts=3, # attempts: 1,2,3
base_delay_seconds=30, # fibonacci delays: 30, 30, 60
jitter_seconds_min=0,
jitter_seconds_max=60,
)
retry = GitHubRateLimitRetry(policy=policy)
ctx = DummyCtx(quiet=True)
runner = FakeRunner(fail_count=2) # fail twice (403), then succeed
# Make jitter deterministic and prevent real sleeping.
with patch("pkgmgr.actions.install.installers.nix.retry.random.randint", return_value=5) as jitter_mock, patch(
"pkgmgr.actions.install.installers.nix.retry.time.sleep"
) as sleep_mock:
res = retry.run_with_retry(ctx, runner, "nix profile install /tmp#default")
# Result should be success on 3rd attempt.
self.assertEqual(res.returncode, 0)
self.assertEqual(runner.calls, 3)
# jitter should be used for each retry sleep (attempt 1->2, attempt 2->3) => 2 sleeps
self.assertEqual(jitter_mock.call_count, 2)
self.assertEqual(sleep_mock.call_count, 2)
# Fibonacci delays for attempts=3: [30, 30, 60]
# sleep occurs after failed attempt 1 and 2, so base delays are 30 and 30
# wait_time = base_delay + jitter(5) => 35, 35
sleep_args = [c.args[0] for c in sleep_mock.call_args_list]
self.assertEqual(sleep_args, [35, 35])
def test_does_not_retry_on_non_403_errors(self) -> None:
"""
Ensure it does not retry when the error is not recognized as GitHub 403/rate limit.
"""
policy = RetryPolicy(max_attempts=7, base_delay_seconds=30)
retry = GitHubRateLimitRetry(policy=policy)
ctx = DummyCtx(quiet=True)
class Non403Runner:
def __init__(self) -> None:
self.calls = 0
def run(self, ctx: DummyCtx, cmd: str, allow_failure: bool) -> RunResult:
self.calls += 1
return RunResult(returncode=1, stdout="", stderr="some other error (simulated)")
runner = Non403Runner()
with patch("pkgmgr.actions.install.installers.nix.retry.time.sleep") as sleep_mock:
res = retry.run_with_retry(ctx, runner, "nix profile install /tmp#default")
self.assertEqual(res.returncode, 1)
self.assertEqual(runner.calls, 1) # no retries
self.assertEqual(sleep_mock.call_count, 0)
if __name__ == "__main__":
unittest.main()