Add retry logic when fetching addons, and save error to cache for user investigation - #493
Add retry logic when fetching addons, and save error to cache for user investigation#493MarcBresson wants to merge 4 commits into
Conversation
A crash during the 6-hourly cache rebuild could permanently drop an addon's cached package.xml/icon for that cycle (since the existing good clone was deleted beforehand). Retry git commands a few times before giving up. If an update ultimately fails, keep the existing good clone. Also retry addons that failed on the previous run first, so a rate-limit-driven failure doesn't always strand the same addons at the tail of a fixed processing order.
for more information, see https://pre-commit.ci
|
sorry for the format issue, ruff is runing when I save files, and it seems like I forgot to undo changes from the test file |
|
Initially I also added an extra step that tried recloning the repo when there was a fetch failure, but I figured it would be redundant so I removed it in the third commit |
| def clone_with_retries(self, url: str, branch: str, target_dir: str) -> None: | ||
| """Attempt a shallow 'git clone' of url/branch into target_dir, retrying up to | ||
| MAX_ATTEMPTS times with a short delay in between. A timeout and a non-zero exit code | ||
| are treated identically: git's exit code doesn't reliably distinguish a transient | ||
| network blip from a permanent error, and retrying a permanent failure a couple of extra | ||
| times is cheap for an unattended job. Before every attempt, any pre-existing target_dir | ||
| is removed, since git clone refuses to run into a non-empty directory and a partial | ||
| checkout can be left behind by a killed or timed-out previous attempt. Raises | ||
| RuntimeError (with git's own stderr appended, if any was captured) if every attempt | ||
| fails; deliberately does not touch self.clone_errors, since callers use this helper for | ||
| two different targets that need different keys.""" |
There was a problem hiding this comment.
No need to spell out in a comment the exact implementation details: in general I try to limit docstrings to a sentence or two. At some point it's easier to just read the code!
There was a problem hiding this comment.
I simplified docstrings to be shorter :) I tried explaining what was considered an error (especially when dealing with subprocesses), and some git behaviours. I'm leaving that to the code and developer's knowledge!
|
also, I did not use logger because the file was using print statements, but I don't know if there is a specific internal policy against the use of logger. If you want, I can replace prints with logger calls |
2 participants
caching errors also allows for failed add-on fetches to be retried first
Mitigates #491