Wait for GHA eval results instead of falling back to local evaluation #451
Conversation
|
Maybe we should silence the warning:
|
|
This implementation is quite naive. We could eventually wait within the function and thus not restarting the fetching process from scratch each time. |
| packages_per_system = self.github_client.get_github_action_eval_result(pr) | ||
| timeout: int = 30 | ||
| while packages_per_system is None: | ||
| print(f"...Results are not (yet) available. Retrying in {timeout}s") |
There was a problem hiding this comment.
Maybe just print this message just once and just add a dot add the end each time it's not finished to indicate progress.
There was a problem hiding this comment.
Good idea ! Done.
nixpkgs_review/review.py
Outdated
| if packages_per_system is not None: | ||
| print("-> Successfully fetched rebuilds: no local evaluation needed") | ||
| packages_per_system = self.github_client.get_github_action_eval_result(pr) | ||
| timeout: int = 30 |
There was a problem hiding this comment.
I'd say 10s here. We have a fair amount of requests we can do our token.
I removed them all. Do you have a better idea ? |
|
|
||
| packages_per_system = self.github_client.get_github_action_eval_result(pr) |
There was a problem hiding this comment.
I believe this also returns None if the evaluation failed. However in this case we shouldn't wait as we would wait forever.
There was a problem hiding this comment.
Should we crash in this case then ?
There was a problem hiding this comment.
This could be another reason why we would want to wait 'within' the github function. I.e. we would wait only in the case where we know that the results are currently computed.
GaetanLepage
force-pushed
the
wait
branch
4 times, most recently
from
471bd4f to
0247365
Compare
| timeout: int = 10 | ||
| print(f"...Results are not (yet) available. Retrying in {timeout}s") | ||
| while packages_per_system is None: | ||
| print(".", end="") |
There was a problem hiding this comment.
This doesn't work. Nothing is printed. If I remove end = "", it works.
There was a problem hiding this comment.
I think it's line buffered. Try flushing sys.stdout.flush()
|
I confirm that printing is fixed: |
|
Should we add a tip: |
The default evaluation mode (
github) attempts at fetching the evaluation results from the GitHub actions result.Those will only be available a few minutes after the PR has been opened as they are not trivial to compute.
Currently, if the results are not available right away,
nixpkgs-reviewfalls back to local evaluation.This PR changes this behavior. Now it will keep re-trying until the results are available.
Note: It is still possible to force local evaluation by using
--eval local.Motivation: Local evaluation is very resource-intensive (especially RAM). It should be an opt-in behavior.
This new approach is more predictable.
Questions:
10sa good value for the timeout ? -> Switched to 30s. Still open to feedback.Ctrl-Ccorrectly (i.e. properly delete the worktree on exit)