Skip to content

Commit

Permalink
Merge pull request #451 from GaetanLepage/wait
Browse files Browse the repository at this point in the history 
Wait for GHA eval results instead of falling back to local evaluation
  • Loading branch information
@Mic92
Mic92 authored Feb 3, 2025
2 parents 110f0b4 + cf8cc7c commit 61784f4
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 29 deletions.
11 changes: 6 additions & 5 deletions nixpkgs_review/builddir.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,11 @@ def __exit__(
os.environ.update(self.environ)

with DisableKeyboardInterrupt():
res = sh(["git", "worktree", "remove", "-f", str(self.worktree_dir)])
if res.returncode != 0:
warn(
f"Failed to remove worktree at {self.worktree_dir}. Please remove it manually. Git failed with: {res.returncode}"
)
if Path.exists(self.worktree_dir / ".git"):
res = sh(["git", "worktree", "remove", "-f", str(self.worktree_dir)])
if res.returncode != 0:
warn(
f"Failed to remove worktree at {self.worktree_dir}. Please remove it manually. Git failed with: {res.returncode}"
)

self.overlay.cleanup()
17 changes: 1 addition & 16 deletions nixpkgs_review/github.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from pathlib import Path
from typing import IO, Any, override

from .utils import System, warn
from .utils import System


def pr_url(pr: int) -> str:
Expand Down Expand Up @@ -175,19 +175,14 @@ def get_github_action_eval_result(
workflow_run["artifacts_url"],
)["artifacts"]

found_comparison = False
for artifact in artifacts:
if artifact["name"] != "comparison":
continue
found_comparison = True
changed_paths: Any = self.get_json_from_artifact(
workflow_id=artifact["id"],
json_filename="changed-paths.json",
)
if changed_paths is None:
warn(
f"Found comparison artifact, but no changed-paths.json in workflow {workflow_run['html_url']}"
)
continue
if (path := changed_paths.get("rebuildsByPlatform")) is not None:
assert isinstance(path, dict)
Expand All @@ -197,14 +192,4 @@ def get_github_action_eval_result(
for system, packages_list in path.items()
}

if not found_comparison:
if workflow_run["status"] == "queued":
warn(
f"Found eval workflow run, but evaluation is still work in progress: {workflow_run['html_url']}"
)
else:
warn(
f"Found eval workflow run, but no comparison artifact in {workflow_run['html_url']}."
)

return None
36 changes: 28 additions & 8 deletions nixpkgs_review/review.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import subprocess
import sys
import tempfile
import time
from dataclasses import dataclass, field
from enum import Enum
from pathlib import Path
Expand Down Expand Up @@ -215,6 +216,7 @@ def build_commit(
Review a local git commit
"""
self.git_worktree(base_commit)
changed_attrs: dict[System, set[str]] = {}

if self.only_packages:
if reviewed_commit is None:
Expand Down Expand Up @@ -264,7 +266,7 @@ def build_commit(
key=system_order_key,
reverse=True,
)
changed_attrs: dict[System, set[str]] = {}
changed_attrs = {}
for system in sorted_systems:
changed_pkgs, removed_pkgs = differences(
base_packages[system], merged_packages[system]
Expand Down Expand Up @@ -312,14 +314,32 @@ def build_pr(self, pr_number: int) -> dict[System, list[Attr]]:
pr = self.github_client.pull_request(pr_number)

packages_per_system: dict[System, set[str]] | None = None
if self.use_github_eval and all(system in PLATFORMS for system in self.systems):
# Attempt to fetch the GitHub actions evaluation result
print("-> Attempting to fetch eval results from GitHub actions")
packages_per_system = self.github_client.get_github_action_eval_result(pr)

if packages_per_system is not None:
print("-> Successfully fetched rebuilds: no local evaluation needed")
if self.use_github_eval:
assert all(system in PLATFORMS for system in self.systems)
print("-> Fetching eval results from GitHub actions")

packages_per_system = self.github_client.get_github_action_eval_result(pr)
if packages_per_system is None:
timeout_s: int = 10
print(f"...Results are not (yet) available. Retrying in {timeout_s}s")
waiting_time_s: int = 0
while packages_per_system is None:
waiting_time_s += timeout_s
print(".", end="")
sys.stdout.flush()
time.sleep(timeout_s)
packages_per_system = (
self.github_client.get_github_action_eval_result(pr)
)
if waiting_time_s > 10 * 60:
warn(
"\nTimeout exceeded: No evaluation seems to be available on GitHub."
"\nLook for an eventual evaluation error issue on the PR web page."
)
sys.exit(1)
print()

print("-> Successfully fetched rebuilds: no local evaluation needed")
else:
packages_per_system = None

Expand Down
10 changes: 10 additions & 0 deletions tests/test_pr.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ def test_pr_local_eval(helpers: Helpers, capfd: pytest.CaptureFixture) -> None:
"pr",
"--remote",
str(nixpkgs.remote),
"--eval",
"local",
"--run",
"exit 0",
"1",
Expand Down Expand Up @@ -63,6 +65,8 @@ def test_pr_local_eval_missing_nom(
"pr",
"--remote",
str(nixpkgs.remote),
"--eval",
"local",
"--run",
"exit 0",
"1",
Expand Down Expand Up @@ -90,6 +94,8 @@ def test_pr_local_eval_without_nom(
"pr",
"--remote",
str(nixpkgs.remote),
"--eval",
"local",
"--run",
"exit 0",
"1",
Expand Down Expand Up @@ -118,6 +124,8 @@ def test_pr_local_eval_with_sandbox(helpers: Helpers) -> None:
"--sandbox",
"--remote",
str(nixpkgs.remote),
"--eval",
"local",
"--run",
"exit 0",
"1",
Expand Down Expand Up @@ -164,6 +172,8 @@ def test_pr_ofborg_eval(mock_urlopen: MagicMock, helpers: Helpers) -> None:
"pr",
"--remote",
str(nixpkgs.remote),
"--eval",
"local",
"--run",
"exit 0",
"37200",
Expand Down

0 comments on commit 61784f4

Please sign in to comment.