From 4fd59bead331082f4a8f7adea920f48288904f6e Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 17 Aug 2023 08:50:05 -0400 Subject: [PATCH 1/2] Retry set_ref up to 5 times This hopefully reduces the rate of these exceptions we see on PRs, but since we don't know the exact cause we'll have to see if it actually works. --- homu/server.py | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/homu/server.py b/homu/server.py index 20502b8..3cde1ad 100644 --- a/homu/server.py +++ b/homu/server.py @@ -37,6 +37,7 @@ from retrying import retry import random import string +import time import bottle bottle.BaseRequest.MEMFILE_MAX = 1024 * 1024 * 10 @@ -690,6 +691,13 @@ def report_build_res(succ, url, builder, state, logger, repo_cfg): ) if state.approved_by and not state.try_: + # The set_ref call below sometimes fails with 422 failed to + # fast forward. We believe this is a spurious error on GitHub's + # side, though it's not entirely clear why. We sleep for 1 + # minute before trying it after setting the status to try to + # increase the likelihood it will work, and also retry the + # set_ref a few times. + time.sleep(60) state.add_comment(comments.BuildCompleted( approved_by=state.approved_by, base_ref=state.base_ref, @@ -698,16 +706,17 @@ def report_build_res(succ, url, builder, state, logger, repo_cfg): )) state.change_labels(LabelEvent.SUCCEED) - def set_ref(): + def set_ref_inner(): utils.github_set_ref(state.get_repo(), 'heads/' + state.base_ref, state.merge_sha) if state.test_on_fork is not None: utils.github_set_ref(state.get_test_on_fork_repo(), 'heads/' + state.base_ref, state.merge_sha, force=True) - try: + + def set_ref(): try: - set_ref() + set_ref_inner() except github3.models.GitHubError: utils.github_create_status( state.get_repo(), @@ -715,14 +724,26 @@ def set_ref(): 'success', '', 'Branch protection bypassed', context='homu') - set_ref() + set_ref_inner() - state.fake_merge(repo_cfg) + error = None + for i in range(0, 5): + try: + set_ref() + state.fake_merge(repo_cfg) + error = None + except github3.models.GitHubError as e: + error = e + pass + if error is None: + break + else: + time.sleep(10) - except github3.models.GitHubError as e: + if error is not None: state.set_status('error') desc = ('Test was successful, but fast-forwarding failed:' - ' {}'.format(e)) + ' {}'.format(error)) utils.github_create_status(state.get_repo(), state.head_sha, 'error', url, desc, context='homu') From 30da5121701ef5bbc4255ec70228fa49aafeb868 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 17 Aug 2023 09:01:33 -0400 Subject: [PATCH 2/2] Fix lints --- homu/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homu/main.py b/homu/main.py index 12a66e4..ab1d0c3 100644 --- a/homu/main.py +++ b/homu/main.py @@ -1668,7 +1668,7 @@ def synchronize(repo_label, repo_cfg, logger, gh, states, repos, db, mergeable_q def process_config(config): # Replace environment variables - if type(config) == str: + if type(config) is str: for var in VARIABLES_RE.findall(config): try: config = config.replace("${"+var+"}", os.environ[var]) @@ -1680,9 +1680,9 @@ def process_config(config): return config # Recursively apply the processing - elif type(config) == list: + elif type(config) is list: return [process_config(item) for item in config] - elif type(config) == dict: + elif type(config) is dict: return {key: process_config(value) for key, value in config.items()} # All other values should be returned as-is else: