From f0be734c0158d2b1cadd9aca8733ddedea1c7e1c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 20 Sep 2018 13:08:02 -0700 Subject: [PATCH 1/2] fix polling processor retry logic after HTTP error --- ldclient/polling.py | 11 +++++------ testing/stub_util.py | 2 ++ testing/test_polling_processor.py | 25 ++++++++++++++----------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ldclient/polling.py b/ldclient/polling.py index 81881d49..dedb430c 100644 --- a/ldclient/polling.py +++ b/ldclient/polling.py @@ -8,18 +8,18 @@ class PollingUpdateProcessor(Thread, UpdateProcessor): - def __init__(self, config, requester, store, ready): + def __init__(self, config, requester, store, ready, override_poll_interval = None): Thread.__init__(self) self.daemon = True - self._config = config self._requester = requester self._store = store self._running = False self._ready = ready + self._interval = config.poll_interval if override_poll_interval is None else override_poll_interval def run(self): if not self._running: - log.info("Starting PollingUpdateProcessor with request interval: " + str(self._config.poll_interval)) + log.info("Starting PollingUpdateProcessor with request interval: " + str(self._interval)) self._running = True while self._running: start_time = time.time() @@ -34,14 +34,13 @@ def run(self): if not is_http_error_recoverable(e.status): self._ready.set() # if client is initializing, make it stop waiting; has no effect if already inited self.stop() - break except Exception as e: log.exception( 'Error: Exception encountered when updating flags. %s' % e) elapsed = time.time() - start_time - if elapsed < self._config.poll_interval: - time.sleep(self._config.poll_interval - elapsed) + if elapsed < self._interval: + time.sleep(self._interval - elapsed) def initialized(self): return self._running and self._ready.is_set() is True and self._store.initialized is True diff --git a/testing/stub_util.py b/testing/stub_util.py index 07e5c2ec..bcb45ef2 100644 --- a/testing/stub_util.py +++ b/testing/stub_util.py @@ -29,8 +29,10 @@ class MockFeatureRequester(FeatureRequester): def __init__(self): self.all_data = {} self.exception = None + self.request_count = 0 def get_all_data(self): + self.request_count += 1 if self.exception is not None: raise self.exception return self.all_data diff --git a/testing/test_polling_processor.py b/testing/test_polling_processor.py index 06bae21d..96e96415 100644 --- a/testing/test_polling_processor.py +++ b/testing/test_polling_processor.py @@ -10,7 +10,6 @@ from ldclient.versioned_data_kind import FEATURES, SEGMENTS from testing.stub_util import MockFeatureRequester, MockResponse -config = Config() pp = None mock_requester = None store = None @@ -27,9 +26,9 @@ def teardown_function(): if pp is not None: pp.stop() -def setup_processor(config): +def setup_processor(config, override_interval = None): global pp - pp = PollingUpdateProcessor(config, mock_requester, store, ready) + pp = PollingUpdateProcessor(config, mock_requester, store, ready, override_interval) pp.start() def test_successful_request_puts_feature_data_in_store(): @@ -47,7 +46,7 @@ def test_successful_request_puts_feature_data_in_store(): "segkey": segment } } - setup_processor(config) + setup_processor(Config()) ready.wait() assert store.get(FEATURES, "flagkey", lambda x: x) == flag assert store.get(SEGMENTS, "segkey", lambda x: x) == segment @@ -57,11 +56,10 @@ def test_successful_request_puts_feature_data_in_store(): def test_general_connection_error_does_not_cause_immediate_failure(): mock_requester.exception = Exception("bad") start_time = time.time() - setup_processor(config) + setup_processor(Config(), 0.1) ready.wait(0.3) - elapsed_time = time.time() - start_time - assert elapsed_time >= 0.2 assert not pp.initialized() + assert mock_requester.request_count >= 2 def test_http_401_error_causes_immediate_failure(): verify_unrecoverable_http_error(401) @@ -78,16 +76,21 @@ def test_http_429_error_does_not_cause_immediate_failure(): def test_http_500_error_does_not_cause_immediate_failure(): verify_recoverable_http_error(500) +def test_http_503_error_does_not_cause_immediate_failure(): + verify_recoverable_http_error(503) + def verify_unrecoverable_http_error(status): mock_requester.exception = UnsuccessfulResponseException(status) - setup_processor(config) - finished = ready.wait(5.0) + setup_processor(Config(), 0.1) + finished = ready.wait(0.5) assert finished assert not pp.initialized() + assert mock_requester.request_count == 1 def verify_recoverable_http_error(status): mock_requester.exception = UnsuccessfulResponseException(status) - setup_processor(config) - finished = ready.wait(0.2) + setup_processor(Config(), 0.1) + finished = ready.wait(0.5) assert not finished assert not pp.initialized() + assert mock_requester.request_count >= 2 From e6cef872a77e8cc09c35da68378c6a493b0dc58e Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 21 Sep 2018 13:30:39 -0700 Subject: [PATCH 2/2] use mock property for tests --- ldclient/polling.py | 10 +++++----- testing/test_polling_processor.py | 22 ++++++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/ldclient/polling.py b/ldclient/polling.py index dedb430c..19ed0a7d 100644 --- a/ldclient/polling.py +++ b/ldclient/polling.py @@ -8,18 +8,18 @@ class PollingUpdateProcessor(Thread, UpdateProcessor): - def __init__(self, config, requester, store, ready, override_poll_interval = None): + def __init__(self, config, requester, store, ready): Thread.__init__(self) self.daemon = True + self._config = config self._requester = requester self._store = store self._running = False self._ready = ready - self._interval = config.poll_interval if override_poll_interval is None else override_poll_interval def run(self): if not self._running: - log.info("Starting PollingUpdateProcessor with request interval: " + str(self._interval)) + log.info("Starting PollingUpdateProcessor with request interval: " + str(self._config.poll_interval)) self._running = True while self._running: start_time = time.time() @@ -39,8 +39,8 @@ def run(self): 'Error: Exception encountered when updating flags. %s' % e) elapsed = time.time() - start_time - if elapsed < self._interval: - time.sleep(self._interval - elapsed) + if elapsed < self._config.poll_interval: + time.sleep(self._config.poll_interval - elapsed) def initialized(self): return self._running and self._ready.is_set() is True and self._store.initialized is True diff --git a/testing/test_polling_processor.py b/testing/test_polling_processor.py index 96e96415..113672f3 100644 --- a/testing/test_polling_processor.py +++ b/testing/test_polling_processor.py @@ -1,6 +1,7 @@ import pytest import threading import time +import mock from ldclient.config import Config from ldclient.feature_store import InMemoryFeatureStore @@ -26,9 +27,9 @@ def teardown_function(): if pp is not None: pp.stop() -def setup_processor(config, override_interval = None): +def setup_processor(config): global pp - pp = PollingUpdateProcessor(config, mock_requester, store, ready, override_interval) + pp = PollingUpdateProcessor(config, mock_requester, store, ready) pp.start() def test_successful_request_puts_feature_data_in_store(): @@ -53,10 +54,13 @@ def test_successful_request_puts_feature_data_in_store(): assert store.initialized assert pp.initialized() -def test_general_connection_error_does_not_cause_immediate_failure(): +# Note that we have to mock Config.poll_interval because Config won't let you set a value less than 30 seconds + +@mock.patch('ldclient.config.Config.poll_interval', new_callable=mock.PropertyMock, return_value=0.1) +def test_general_connection_error_does_not_cause_immediate_failure(ignore_mock): mock_requester.exception = Exception("bad") start_time = time.time() - setup_processor(Config(), 0.1) + setup_processor(Config()) ready.wait(0.3) assert not pp.initialized() assert mock_requester.request_count >= 2 @@ -79,17 +83,19 @@ def test_http_500_error_does_not_cause_immediate_failure(): def test_http_503_error_does_not_cause_immediate_failure(): verify_recoverable_http_error(503) -def verify_unrecoverable_http_error(status): +@mock.patch('ldclient.config.Config.poll_interval', new_callable=mock.PropertyMock, return_value=0.1) +def verify_unrecoverable_http_error(status, ignore_mock): mock_requester.exception = UnsuccessfulResponseException(status) - setup_processor(Config(), 0.1) + setup_processor(Config()) finished = ready.wait(0.5) assert finished assert not pp.initialized() assert mock_requester.request_count == 1 -def verify_recoverable_http_error(status): +@mock.patch('ldclient.config.Config.poll_interval', new_callable=mock.PropertyMock, return_value=0.1) +def verify_recoverable_http_error(status, ignore_mock): mock_requester.exception = UnsuccessfulResponseException(status) - setup_processor(Config(), 0.1) + setup_processor(Config()) finished = ready.wait(0.5) assert not finished assert not pp.initialized()