diff --git a/attract_server/subversion.py b/attract_server/subversion.py index b9034ef..6284fc6 100644 --- a/attract_server/subversion.py +++ b/attract_server/subversion.py @@ -7,6 +7,7 @@ import re import attr import blinker import svn.remote +import svn.common from . import attrs_extra @@ -31,25 +32,34 @@ class CommitLogObserver(object): self._log.debug('%s: fetch_and_observe()', self) - svn_log = self.svn_client.log_default( - revision_from=self.last_seen_revision + 1) + try: + svn_log = self.svn_client.log_default(revision_from=self.last_seen_revision + 1) + for log_entry in svn_log: + self._log.debug('- %r', log_entry) - for log_entry in svn_log: - self._log.debug('- %r', log_entry) + # assumption: revisions are always logged in strictly increasing order. + self.last_seen_revision = log_entry.revision - # assumption: revisions are always logged in strictly increasing order. - self.last_seen_revision = log_entry.revision + self._parse_log_entry(log_entry) + except svn.common.SvnCommandError: + # The SVN library just raises a SvnCommandError when something goes wrong, + # without any structured indication of the error. There isn't much else + # we can do, except to log the error and return. + self._log.exception('Error calling self.svn_client.log_default()') + return - # Log entries can be None. - if not log_entry.msg: - continue + def _parse_log_entry(self, log_entry): + """Parses the commit log to see if there are any task markers.""" - # Parse the commit log to see if there are any task markers. - lines = log_entry.msg.split('\n', 1) - first_line = lines[0] - for match in task_marker_re.finditer(first_line): - task_id = match.group('task_id') + # Log entries can be None. + if not log_entry.msg: + return - task_logged.send(self, - task_id=task_id, - log_entry=log_entry) + # Parse the commit log to see if there are any task markers. + lines = log_entry.msg.split('\n', 1) + first_line = lines[0] + for match in task_marker_re.finditer(first_line): + task_id = match.group('task_id') + + # Send a Blinker signal for each observed task identifier. + task_logged.send(self, task_id=task_id, log_entry=log_entry) diff --git a/requirements.txt b/requirements.txt index 91b7d98..a166339 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,10 @@ # Primary requirements: pillar attrs==16.1.0 -svn==0.3.42 +# Use Sybren's branch, until this pull request is merged: +# https://github.com/dsoprea/PySvn/pull/43 +-e git+https://github.com/sybrenstuvel/PySvn.git@sybren-svncommanderror#egg=svn + # Testing requirements: pytest==3.0.1 diff --git a/tests/test_subversion.py b/tests/test_subversion.py index 3af05fd..7b982a9 100644 --- a/tests/test_subversion.py +++ b/tests/test_subversion.py @@ -9,6 +9,7 @@ import unittest from dateutil.tz import tzutc import mock +import svn.common import logging_config from attract_server import subversion @@ -122,3 +123,18 @@ class TestCommitLogObserver(unittest.TestCase): blinks[1]) self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'task_id': 'T4433'}, blinks[2]) + + def test_svn_error(self): + """SVN errors should not crash the observer.""" + + self.mock_client.log_default = mock.Mock(name='log_default', + side_effect=svn.common.SvnCommandError('unittest')) + + record_blink = mock.Mock(name='record_blink', + spec={'__name__': 'record_blink'}) + subversion.task_logged.connect(record_blink) + + self.observer.fetch_and_observe() + + record_blink.assert_not_called() + self.mock_client.log_default.assert_called_once()