From dced0aea28965750183e62af572d6b92fd9070da Mon Sep 17 00:00:00 2001 From: Ilya Shakhat Date: Thu, 30 Jun 2016 15:51:43 +0300 Subject: [PATCH] Fail on Gerrit errors Do not try to recover in case of Gerrit errors. It appears that in certain circumstances retries cause Gerrit to ban the user. Change-Id: I3ca3aa782eb8d2353a61ca504603e2935f8396d1 --- .../processor/default_data_processor.py | 13 +++++----- stackalytics/processor/rcs.py | 25 ++++++++++--------- stackalytics/tests/unit/test_rcs.py | 7 +++--- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/stackalytics/processor/default_data_processor.py b/stackalytics/processor/default_data_processor.py index f81b24145..71324226d 100644 --- a/stackalytics/processor/default_data_processor.py +++ b/stackalytics/processor/default_data_processor.py @@ -75,10 +75,9 @@ def _retrieve_project_list_from_gerrit(project_source): project_list = gerrit_inst.get_project_list() gerrit_inst.close() - except Exception as e: - LOG.exception(e) - LOG.warning('Fail to retrieve list of projects. Keep it unmodified') - return + except rcs.RcsException: + LOG.error('Failed to retrieve list of projects') + raise organization = project_source['organization'] LOG.debug('Get list of projects for organization %s', organization) @@ -108,9 +107,9 @@ def _retrieve_project_list_from_github(project_source): try: github_repos = github.get_organization(organization).get_repos() except Exception as e: - LOG.exception(e) - LOG.warning('Fail to retrieve list of projects. Keep it unmodified') - return + LOG.error('Failed to retrieve list of projects from GitHub: %s', + e, exc_info=True) + raise for repo in github_repos: yield { diff --git a/stackalytics/processor/rcs.py b/stackalytics/processor/rcs.py index 82dbb44e1..a81435bd5 100644 --- a/stackalytics/processor/rcs.py +++ b/stackalytics/processor/rcs.py @@ -28,6 +28,10 @@ PAGE_LIMIT = 100 REQUEST_COUNT_LIMIT = 20 +class RcsException(Exception): + pass + + class Rcs(object): def __init__(self): pass @@ -56,7 +60,7 @@ class Gerrit(Rcs): if not self.port: self.port = DEFAULT_PORT else: - raise Exception('Invalid rcs uri %s' % uri) + raise RcsException('Invalid rcs uri %s' % uri) self.key_filename = None self.username = None @@ -71,7 +75,7 @@ class Gerrit(Rcs): self.key_filename = kwargs.get('key_filename') self.username = kwargs.get('username') - return self._connect() + self._connect() def _connect(self): try: @@ -79,13 +83,12 @@ class Gerrit(Rcs): key_filename=self.key_filename, username=self.username) LOG.debug('Successfully connected to Gerrit') - return True except Exception as e: LOG.error('Failed to connect to gerrit %(host)s:%(port)s. ' - 'Error: %(err)s', {'host': self.hostname, - 'port': self.port, 'err': e}) - LOG.exception(e) - return False + 'Error: %(err)s', + {'host': self.hostname, 'port': self.port, 'err': e}, + exc_info=True) + raise RcsException('Failed to connect to gerrit: %s' % e) def _get_cmd(self, project_organization, module, branch, age=0, status=None, limit=PAGE_LIMIT, grab_comments=False): @@ -113,10 +116,8 @@ class Gerrit(Rcs): return self.client.exec_command(cmd) except Exception as e: LOG.error('Error %(error)s while execute command %(cmd)s', - {'error': e, 'cmd': cmd}) - LOG.exception(e) - self.request_count = REQUEST_COUNT_LIMIT - return False + {'error': e, 'cmd': cmd}, exc_info=True) + raise RcsException('Failed to execute command: %s', cmd) def _poll_reviews(self, project_organization, module, branch, last_retrieval_time, status=None, grab_comments=False): @@ -163,7 +164,7 @@ class Gerrit(Rcs): def get_project_list(self): exec_result = self._exec_command('gerrit ls-projects') if not exec_result: - raise Exception("Unable to retrieve list of projects from gerrit.") + raise RcsException("Gerrit returned no projects") stdin, stdout, stderr = exec_result result = [line.strip() for line in stdout] diff --git a/stackalytics/tests/unit/test_rcs.py b/stackalytics/tests/unit/test_rcs.py index b5b138cb5..f2f343efd 100644 --- a/stackalytics/tests/unit/test_rcs.py +++ b/stackalytics/tests/unit/test_rcs.py @@ -41,9 +41,8 @@ class TestRcs(testtools.TestCase): mock_client.connect = mock_connect gerrit = rcs.Gerrit('gerrit://review.openstack.org') - setup_result = gerrit.setup(username='user', key_filename='key') + gerrit.setup(username='user', key_filename='key') - self.assertTrue(setup_result) mock_connect.assert_called_once_with( 'review.openstack.org', port=rcs.DEFAULT_PORT, key_filename='key', username='user') @@ -58,9 +57,9 @@ class TestRcs(testtools.TestCase): mock_connect.side_effect = Exception gerrit = rcs.Gerrit('gerrit://review.openstack.org') - setup_result = gerrit.setup(username='user', key_filename='key') + self.assertRaises(rcs.RcsException, gerrit.setup, + username='user', key_filename='key') - self.assertFalse(setup_result) mock_connect.assert_called_once_with( 'review.openstack.org', port=rcs.DEFAULT_PORT, key_filename='key', username='user')