Improve GitHub client retry behaviour

.github/workflows/ci-cd.yml

@@ -106,6 +106,11 @@ jobs:
   publish-docker-image:
     name: Publish Unit Test Results (Docker Image)
     needs: test
+    # we run the action from this branch whenever we can (when it runs in our repo's context)
+    if: >
+      always() &&
+      github.event.sender.login != 'dependabot[bot]' &&
+      ( github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository )
     runs-on: ubuntu-latest
 
     steps:

publish/publisher.py

@@ -1,12 +1,20 @@
+import logging
+import re
 from dataclasses import dataclass
+from typing import List, Any, Optional, Tuple, Mapping
 
 from github import Github
 from github.CheckRun import CheckRun
 from github.CheckRunAnnotation import CheckRunAnnotation
 from github.PullRequest import PullRequest
 
 from github_action import GithubAction
-from publish import *
+from publish import hide_comments_mode_orphaned, hide_comments_mode_all_but_latest, \
+    get_stats_from_digest, digest_prefix, get_short_summary, get_long_summary_md, \
+    get_long_summary_with_digest_md, get_error_annotations, get_case_annotations, \
+    get_all_tests_list_annotation, get_skipped_tests_list_annotation, get_all_tests_list, \
+    get_skipped_tests_list, all_tests_list, skipped_tests_list, pull_request_build_mode_merge, \
+    Annotation, SomeTestChanges
 from unittestresults import UnitTestCaseResults, UnitTestRunResults, get_stats_delta
 
 

publish_unit_test_results.py

@@ -29,6 +29,14 @@ def get_conclusion(parsed: ParsedUnitTestResults, fail_on_failures, fail_on_erro
     return 'success'
 
 
+def get_github(token: str, url: str, retries: int, backoff_factor: float) -> github.Github:
+    retry = Retry(total=retries,
+                  backoff_factor=backoff_factor,
+                  allowed_methods=Retry.DEFAULT_ALLOWED_METHODS.union({'GET', 'POST'}),
+                  status_forcelist=range(500, 600))
+    return github.Github(login_or_token=token, base_url=url, retry=retry)
+
+
 def main(settings: Settings) -> None:
     gha = GithubAction()
 
@@ -63,8 +71,7 @@ def main(settings: Settings) -> None:
     conclusion = get_conclusion(parsed, fail_on_failures=settings.fail_on_failures, fail_on_errors=settings.fail_on_errors)
 
     # publish the delta stats
-    retry = Retry(total=10, backoff_factor=1)
-    gh = github.Github(login_or_token=settings.token, base_url=settings.api_url, retry=retry)
+    gh = get_github(token=settings.token, url=settings.api_url, retries=10, backoff_factor=1)
     Publisher(settings, gh, gha).publish(stats, results.case_results, conclusion)
 
 

requirements.txt

@@ -1,3 +1,5 @@
+# dataclasses does not exist in Python3.6, needed as dependency
+dataclasses
 junitparser==1.6.1
 PyGithub==1.54.1
-dataclasses
+urllib3==1.26.4

test/requirements.txt

@@ -1,3 +1,6 @@
+flask
 mock
 pytest
 pyyaml>=5.1
+requests
+urllib3<2.0.0

test/test_github.py

@@ -0,0 +1,128 @@
+import logging
+import sys
+import time
+import unittest
+from multiprocessing import Process
+
+import requests.exceptions
+from flask import Flask
+
+from publish_unit_test_results import get_github
+
+
+@unittest.skipIf(sys.platform != 'linux', 'Pickling the mock REST endpoint only works Linux')
+class TestGitHub(unittest.TestCase):
+
+    base_url = f'http://localhost:12380/api'
+    gh = get_github('login or token', base_url, retries=1, backoff_factor=0.1)
+
+    @classmethod
+    def start_api(cls, app: Flask) -> Process:
+        def run():
+            app.run(host='localhost', port=12380)
+
+        server = Process(target=run)
+        server.start()
+        attempt = 0
+        while attempt < 100:
+            try:
+                attempt += 1
+                requests.get('http://localhost:12380/health')
+                return server
+            except requests.exceptions.ConnectionError as e:
+                if attempt % 10 == 0:
+                    logging.warning(f'mock api server is not up yet, tried {attempt} times: {str(e)}')
+                time.sleep(0.01)
+        cls.stop_api(server)
+        raise RuntimeError('Failed to start mock api server, could not connect to health endpoint')
+
+    @staticmethod
+    def stop_api(server: Process) -> None:
+        server.terminate()
+        server.join(2)
+
+    def test_github_get_retry(self):
+        for status in [500, 502, 503, 504]:
+            with self.subTest(status=status):
+                app = Flask(self.test_github_post_retry.__name__)
+
+                @app.route('/health')
+                def health():
+                    return {'health': 'alive'}
+
+                @app.route('/api/repos/<owner>/<repo>')
+                def repo(owner: str, repo: str):
+                    return 'null', status
+
+                server = self.start_api(app)
+                try:
+                    with self.assertRaises(requests.exceptions.RetryError) as context:
+                        self.gh.get_repo('owner/repo')
+                    self.assertIn(f"Max retries exceeded with url: /api/repos/owner/repo", context.exception.args[0].args[0])
+                    self.assertIn(f"Caused by ResponseError('too many {status} error responses'", context.exception.args[0].args[0])
+
+                finally:
+                    self.stop_api(server)
+
+    def test_github_post_retry(self):
+        for status in [500, 502, 503, 504]:
+            with self.subTest(status=status):
+                app = Flask(self.test_github_post_retry.__name__)
+
+                @app.route('/health')
+                def health():
+                    return {'health': 'alive'}
+
+                @app.route('/api/repos/<owner>/<repo>')
+                def repo(owner: str, repo: str):
+                    return {'id': 1234, 'name': repo, 'full_name': '/'.join([owner, repo]), 'url': '/'.join([self.base_url, 'repos', owner, repo])}
+
+                @app.route('/api/repos/<owner>/<repo>/check-runs', methods=['POST'])
+                def check_runs(owner: str, repo: str):
+                    return 'null', status
+
+                @app.route('/api/repos/<owner>/<repo>/pulls/<int:number>')
+                def pull(owner: str, repo: str, number: int):
+                    return {'id': 12345, 'number': number, 'issue_url': '/'.join([self.base_url, 'repos', owner, repo, 'issues', str(number)])}
+
+                @app.route('/api/repos/<owner>/<repo>/issues/<int:number>/comments', methods=['POST'])
+                def comment(owner: str, repo: str, number: int):
+                    return 'null', status
+
+                @app.route('/api/graphql', methods=['POST'])
+                def graphql():
+                    return 'null', status
+
+                server = self.start_api(app)
+                try:
+                    repo = self.gh.get_repo('owner/repo')
+                    expected = {'full_name': 'owner/repo', 'id': 1234, 'name': 'repo', 'url': 'http://localhost:12380/api/repos/owner/repo'}
+                    self.assertEqual(expected, repo.raw_data)
+
+                    with self.assertRaises(requests.exceptions.RetryError) as context:
+                        repo.create_check_run(name='check_name',
+                                              head_sha='sha',
+                                              status='completed',
+                                              conclusion='success',
+                                              output={})
+                    self.assertIn(f"Max retries exceeded with url: /api/repos/owner/repo/check-runs", context.exception.args[0].args[0])
+                    self.assertIn(f"Caused by ResponseError('too many {status} error responses'", context.exception.args[0].args[0])
+
+                    pr = repo.get_pull(1)
+                    expected = {'id': 12345, 'number': 1, 'issue_url': 'http://localhost:12380/api/repos/owner/repo/issues/1'}
+                    self.assertEqual(expected, pr.raw_data)
+
+                    with self.assertRaises(requests.exceptions.RetryError) as context:
+                        pr.create_issue_comment('issue comment body')
+                    self.assertIn(f"Max retries exceeded with url: /api/repos/owner/repo/issues/1/comments", context.exception.args[0].args[0])
+                    self.assertIn(f"Caused by ResponseError('too many {status} error responses'", context.exception.args[0].args[0])
+
+                    with self.assertRaises(requests.exceptions.RetryError) as context:
+                        self.gh._Github__requester.requestJsonAndCheck(
+                            "POST", '/'.join([self.base_url, 'graphql']), input={}
+                        )
+                    self.assertIn(f"Max retries exceeded with url: /api/graphql", context.exception.args[0].args[0])
+                    self.assertIn(f"Caused by ResponseError('too many {status} error responses'", context.exception.args[0].args[0])
+
+                finally:
+                    self.stop_api(server)