diff --git a/runbot/models/build.py b/runbot/models/build.py index 07f56f84..aa2dd566 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -858,10 +858,13 @@ class runbot_build(models.Model): """Notify each repo with a status""" self.ensure_one() if self.config_id.update_github_state: - commits = {(b.repo_id, b.name) for b in self.search([('name', '=', self.name)])} - for repo, commit_hash in commits: - _logger.debug("github updating %s status %s to %s in repo %s", status['context'], commit_hash, status['state'], repo.name) - repo._github('/repos/:owner/:repo/statuses/%s' % commit_hash, status, ignore_errors=True) + repos = {b.repo_id for b in self.search([('name', '=', self.name)])} + for repo in repos: + _logger.debug("github updating %s status %s to %s in repo %s", status['context'], self.name, status['state'], repo.name) + try: + repo._github('/repos/:owner/:repo/statuses/%s' % self.name, status, ignore_errors=True) + except Exception: + self._log('_github_status_notify_all', 'Status notification failed for "%s" in repo "%s"' % (self.name, repo.name)) def _github_status(self): """Notify github of failed/successful builds""" diff --git a/runbot/models/repo.py b/runbot/models/repo.py index a6b238d5..f090004a 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -123,31 +123,39 @@ class runbot_repo(models.Model): return False return True - def _github(self, url, payload=None, ignore_errors=False): + def _github(self, url, payload=None, ignore_errors=False, nb_tries=2): """Return a http request to be sent to github""" for repo in self: if not repo.token: return - try: - match_object = re.search('([^/]+)/([^/]+)/([^/.]+(.git)?)', repo.base) - if match_object: - url = url.replace(':owner', match_object.group(2)) - url = url.replace(':repo', match_object.group(3)) - url = 'https://api.%s%s' % (match_object.group(1), url) - session = requests.Session() - session.auth = (repo.token, 'x-oauth-basic') - session.headers.update({'Accept': 'application/vnd.github.she-hulk-preview+json'}) - if payload: - response = session.post(url, data=json.dumps(payload)) - else: - response = session.get(url) - response.raise_for_status() - return response.json() - except Exception: - if ignore_errors: - _logger.exception('Ignored github error %s %r', url, payload) - else: - raise + match_object = re.search('([^/]+)/([^/]+)/([^/.]+(.git)?)', repo.base) + if match_object: + url = url.replace(':owner', match_object.group(2)) + url = url.replace(':repo', match_object.group(3)) + url = 'https://api.%s%s' % (match_object.group(1), url) + session = requests.Session() + session.auth = (repo.token, 'x-oauth-basic') + session.headers.update({'Accept': 'application/vnd.github.she-hulk-preview+json'}) + try_count = 0 + while try_count < nb_tries: + try: + if payload: + response = session.post(url, data=json.dumps(payload)) + else: + response = session.get(url) + response.raise_for_status() + if try_count > 0: + _logger.info('Success after %s tries' % (try_count + 1)) + return response.json() + except Exception as e: + try_count += 1 + if try_count < nb_tries: + time.sleep(2) + else: + if ignore_errors: + _logger.exception('Ignored github error %s %r (try %s/%s)' % (url, payload, try_count + 1, nb_tries)) + else: + raise def _get_fetch_head_time(self): self.ensure_one() diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py index daffef20..9a395492 100644 --- a/runbot/tests/test_repo.py +++ b/runbot/tests/test_repo.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- +import datetime from unittest import skip -from unittest.mock import patch +from unittest.mock import patch, Mock from odoo.tests import common import logging import odoo @@ -152,6 +153,30 @@ class Test_Repo(common.TransactionCase): _logger.info('Create pending builds took: %ssec', (time.time() - inserted_time)) + def test_github(self): + """ Test different github responses or failures""" + repo = self.Repo.create({'name': 'bla@example.com:foo/foo'}) + self.assertEqual(repo._github('/repos/:owner/:repo/statuses/abcdef', dict(), ignore_errors=True), None, 'A repo without token should return None') + repo.token = 'abc' + with patch('odoo.addons.runbot.models.repo.requests.Session') as mock_session: + with self.assertRaises(Exception, msg='should raise an exception with ignore_errors=False'): + mock_session.return_value.post.side_effect = Exception('301: Bad gateway') + repo._github('/repos/:owner/:repo/statuses/abcdef', {'foo': 'bar'}, ignore_errors=False) + + mock_session.return_value.post.reset_mock() + with self.assertLogs(logger='odoo.addons.runbot.models.repo') as assert_log: + repo._github('/repos/:owner/:repo/statuses/abcdef', {'foo': 'bar'}, ignore_errors=True) + self.assertIn('Ignored github error', assert_log.output[0]) + + self.assertEqual(2, mock_session.return_value.post.call_count, "_github method should try two times by default") + + mock_session.return_value.post.reset_mock() + mock_session.return_value.post.side_effect = [Exception('301: Bad gateway'), Mock()] + with self.assertLogs(logger='odoo.addons.runbot.models.repo') as assert_log: + repo._github('/repos/:owner/:repo/statuses/abcdef', {'foo': 'bar'}, ignore_errors=True) + self.assertIn('Success after 2 tries', assert_log.output[0]) + + self.assertEqual(2, mock_session.return_value.post.call_count, "_github method should try two times by default") class Test_Repo_Scheduler(common.TransactionCase):