Skip to content

Commit 7f359df

Browse files
committed
add retries on exceptions
1 parent 51e63ae commit 7f359df

3 files changed

Lines changed: 88 additions & 12 deletions

File tree

splitio/push/manager.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ def update_workers_status(self, enabled):
6666
"""
6767
self._processor.update_workers_status(enabled)
6868

69-
7069
def start(self):
7170
"""Start a new connection if not already running."""
7271
if self._running:

splitio/sync/synchronizer.py

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,29 +235,51 @@ def synchronize_segment(self, segment_name, till):
235235
:type till: int
236236
"""
237237
_LOGGER.debug('Synchronizing segment %s', segment_name)
238-
return self._split_synchronizers.segment_sync.synchronize_segment(segment_name, till)
238+
success = self._split_synchronizers.segment_sync.synchronize_segment(segment_name, till)
239+
if not success:
240+
_LOGGER.error('Failed to sync some segments.')
241+
return success
239242

240243
def synchronize_splits(self, till):
241244
"""
242245
Synchronize all splits.
243246
244247
:param till: to fetch
245248
:type till: int
249+
250+
:returns: whether the synchronization was successful or not.
251+
:rtype: bool
246252
"""
247253
_LOGGER.debug('Starting splits synchronization')
248-
return self._split_synchronizers.split_sync.synchronize_splits(till)
249-
250-
def sync_all(self):
251-
"""Synchronize all split data."""
252254
try:
253-
self.synchronize_splits(None)
255+
self._split_synchronizers.split_sync.synchronize_splits(till)
256+
return True
254257
except APIException:
255258
_LOGGER.error('Failed syncing splits')
256259
_LOGGER.debug('Error: ', exc_info=True)
260+
return False
257261

258-
if not self._synchronize_segments():
259-
_LOGGER.error('Failed syncing segments')
260-
_LOGGER.debug('Error: ', exc_info=True)
262+
def sync_all(self):
263+
"""Synchronize all split data."""
264+
attempts = 3
265+
while attempts > 0:
266+
try:
267+
if not self.synchronize_splits(None):
268+
attempts -= 1
269+
continue
270+
271+
if not self._synchronize_segments():
272+
attempts -= 1
273+
continue
274+
275+
# All is good
276+
return
277+
except Exception as exc: # pylint:disable=broad-except
278+
attempts -= 1
279+
_LOGGER.error("Exception caught when trying to sync all data: %s", str(exc))
280+
_LOGGER.debug('Error: ', exc_info=True)
281+
282+
_LOGGER.error("Could not correctly synchronize splits and segments after 3 attempts.")
261283

262284
def shutdown(self, blocking):
263285
"""

tests/sync/test_synchronizer.py

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ def run(x):
3333
mocker.Mock(), mocker.Mock(), mocker.Mock())
3434
sychronizer = Synchronizer(split_synchronizers, mocker.Mock(spec=SplitTasks))
3535

36-
with pytest.raises(APIException):
37-
sychronizer.synchronize_splits(None)
36+
sychronizer.synchronize_splits(None) # APIExceptions are handled locally and should not be propagated!
3837

3938
sychronizer.sync_all() # sync_all should not throw!
4039

@@ -209,3 +208,59 @@ def stop_mock_2():
209208
assert len(impression_count_task.stop.mock_calls) == 1
210209
assert len(event_task.stop.mock_calls) == 1
211210
assert len(telemetry_task.stop.mock_calls) == 1
211+
212+
def test_sync_all_ok(self, mocker):
213+
"""Test that 3 attempts are done before failing."""
214+
split_synchronizers = mocker.Mock(spec=SplitSynchronizers)
215+
counts = {'splits': 0, 'segments': 0}
216+
217+
def sync_splits(*_):
218+
"""Sync Splits."""
219+
counts['splits'] += 1
220+
return True
221+
222+
def sync_segments(*_):
223+
"""Sync Segments."""
224+
counts['segments'] += 1
225+
return True
226+
227+
split_synchronizers.split_sync.synchronize_splits.side_effect = sync_splits
228+
split_synchronizers.segment_sync.synchronize_segments.side_effect = sync_segments
229+
split_tasks = mocker.Mock(spec=SplitTasks)
230+
synchronizer = Synchronizer(split_synchronizers, split_tasks)
231+
232+
synchronizer.sync_all()
233+
assert counts['splits'] == 1
234+
assert counts['segments'] == 1
235+
236+
def test_sync_all_attempts(self, mocker):
237+
"""Test that 3 attempts are done before failing."""
238+
split_synchronizers = mocker.Mock(spec=SplitSynchronizers)
239+
counts = {'splits': 0, 'segments': 0}
240+
def sync_splits(*_):
241+
"""Sync Splits."""
242+
counts['splits'] += 1
243+
raise Exception('sarasa')
244+
245+
split_synchronizers.split_sync.synchronize_splits.side_effect = sync_splits
246+
split_tasks = mocker.Mock(spec=SplitTasks)
247+
synchronizer = Synchronizer(split_synchronizers, split_tasks)
248+
249+
synchronizer.sync_all()
250+
assert counts['splits'] == 3
251+
252+
def test_sync_all_attempts(self, mocker):
253+
"""Test that 3 attempts are done before failing."""
254+
split_synchronizers = mocker.Mock(spec=SplitSynchronizers)
255+
counts = {'splits': 0, 'segments': 0}
256+
def sync_segments(*_):
257+
"""Sync Splits."""
258+
counts['segments'] += 1
259+
raise Exception('sarasa')
260+
261+
split_synchronizers.segment_sync.synchronize_segments.side_effect = sync_segments
262+
split_tasks = mocker.Mock(spec=SplitTasks)
263+
synchronizer = Synchronizer(split_synchronizers, split_tasks)
264+
265+
synchronizer.sync_all()
266+
assert counts['segments'] == 3

0 commit comments

Comments
 (0)