[FSSDK-11212] Update code to retry web API calls for fetching datafile and pushing events (#445)

* Update code to retry web API calls for fetching datafile and pushing events

* Fix linting issues

* Remove print statements

* Fix up 'retries' member

* Stub out requests.Session.get instead of requests.get

* Update tests

* Fix mypy error and linting error

* Update for tests

* Update

* Update optimizely/event_dispatcher.py

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

* Update event dispatch to try three times to send events

* Update changelog and version number

* Update version number

* Remove changelog and version update

---------

Co-authored-by: Paul V Craven <paul.craven@optimizley.com>
Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

Author: 3 people

optimizely/config_manager.py

@@ -19,6 +19,8 @@
 import threading
 from requests import codes as http_status_codes
 from requests import exceptions as requests_exceptions
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
 
 from . import exceptions as optimizely_exceptions
 from . import logger as optimizely_logger
@@ -200,6 +202,7 @@ def __init__(
         error_handler: Optional[BaseErrorHandler] = None,
         notification_center: Optional[NotificationCenter] = None,
         skip_json_validation: Optional[bool] = False,
+        retries: Optional[int] = 3,
     ):
         """ Initialize config manager. One of sdk_key or datafile has to be set to be able to use.
 
@@ -222,6 +225,7 @@ def __init__(
                                   JSON schema validation will be performed.
 
         """
+        self.retries = retries
         self._config_ready_event = threading.Event()
         super().__init__(
             datafile=datafile,
@@ -391,9 +395,18 @@ def fetch_datafile(self) -> None:
             request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified
 
         try:
-            response = requests.get(
-                self.datafile_url, headers=request_headers, timeout=enums.ConfigManager.REQUEST_TIMEOUT,
-            )
+            session = requests.Session()
+
+            retries = Retry(total=self.retries,
+                            backoff_factor=0.1,
+                            status_forcelist=[500, 502, 503, 504])
+            adapter = HTTPAdapter(max_retries=retries)
+
+            session.mount('http://', adapter)
+            session.mount("https://", adapter)
+            response = session.get(self.datafile_url,
+                                   headers=request_headers,
+                                   timeout=enums.ConfigManager.REQUEST_TIMEOUT)
         except requests_exceptions.RequestException as err:
             self.logger.error(f'Fetching datafile from {self.datafile_url} failed. Error: {err}')
             return
@@ -475,9 +488,18 @@ def fetch_datafile(self) -> None:
             request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified
 
         try:
-            response = requests.get(
-                self.datafile_url, headers=request_headers, timeout=enums.ConfigManager.REQUEST_TIMEOUT,
-            )
+            session = requests.Session()
+
+            retries = Retry(total=self.retries,
+                            backoff_factor=0.1,
+                            status_forcelist=[500, 502, 503, 504])
+            adapter = HTTPAdapter(max_retries=retries)
+
+            session.mount('http://', adapter)
+            session.mount("https://", adapter)
+            response = session.get(self.datafile_url,
+                                   headers=request_headers,
+                                   timeout=enums.ConfigManager.REQUEST_TIMEOUT)
         except requests_exceptions.RequestException as err:
             self.logger.error(f'Fetching datafile from {self.datafile_url} failed. Error: {err}')
             return

optimizely/event_dispatcher.py

@@ -17,6 +17,8 @@
 
 import requests
 from requests import exceptions as request_exception
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
 
 from . import event_builder
 from .helpers.enums import HTTPVerbs, EventDispatchConfig
@@ -44,11 +46,21 @@ def dispatch_event(event: event_builder.Event) -> None:
       event: Object holding information about the request to be dispatched to the Optimizely backend.
     """
         try:
+            session = requests.Session()
+
+            retries = Retry(total=EventDispatchConfig.RETRIES,
+                            backoff_factor=0.1,
+                            status_forcelist=[500, 502, 503, 504])
+            adapter = HTTPAdapter(max_retries=retries)
+
+            session.mount('http://', adapter)
+            session.mount("https://", adapter)
+
             if event.http_verb == HTTPVerbs.GET:
-                requests.get(event.url, params=event.params,
-                             timeout=EventDispatchConfig.REQUEST_TIMEOUT).raise_for_status()
+                session.get(event.url, params=event.params,
+                            timeout=EventDispatchConfig.REQUEST_TIMEOUT).raise_for_status()
             elif event.http_verb == HTTPVerbs.POST:
-                requests.post(
+                session.post(
                     event.url, data=json.dumps(event.params), headers=event.headers,
                     timeout=EventDispatchConfig.REQUEST_TIMEOUT,
                 ).raise_for_status()

optimizely/helpers/enums.py

@@ -198,6 +198,7 @@ class VersionType:
 class EventDispatchConfig:
     """Event dispatching configs."""
     REQUEST_TIMEOUT: Final = 10
+    RETRIES: Final = 3
 
 
 class OdpEventApiConfig:

optimizely/helpers/validator.py

@@ -276,8 +276,9 @@ def is_finite_number(value: Any) -> bool:
         if math.isnan(value) or math.isinf(value):
             return False
 
-    if abs(value) > (2 ** 53):
-        return False
+    if isinstance(value, (int, float)):
+        if abs(value) > (2 ** 53):
+            return False
 
     return True
 

tests/test_config_manager.py

@@ -218,7 +218,7 @@ def test_get_config_blocks(self):
         self.assertEqual(1, round(end_time - start_time))
 
 
-@mock.patch('requests.get')
+@mock.patch('requests.Session.get')
 class PollingConfigManagerTest(base.BaseTest):
     def test_init__no_sdk_key_no_datafile__fails(self, _):
         """ Test that initialization fails if there is no sdk_key or datafile provided. """
@@ -379,7 +379,7 @@ def test_fetch_datafile(self, _):
         test_response.status_code = 200
         test_response.headers = test_headers
         test_response._content = test_datafile
-        with mock.patch('requests.get', return_value=test_response) as mock_request:
+        with mock.patch('requests.Session.get', return_value=test_response) as mock_request:
             project_config_manager = config_manager.PollingConfigManager(sdk_key=sdk_key)
             project_config_manager.stop()
 
@@ -392,7 +392,7 @@ def test_fetch_datafile(self, _):
         self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig)
 
         # Call fetch_datafile again and assert that request to URL is with If-Modified-Since header.
-        with mock.patch('requests.get', return_value=test_response) as mock_requests:
+        with mock.patch('requests.Session.get', return_value=test_response) as mock_requests:
             project_config_manager._initialize_thread()
             project_config_manager.start()
             project_config_manager.stop()
@@ -421,7 +421,7 @@ def raise_for_status(self):
         test_response.headers = test_headers
         test_response._content = test_datafile
 
-        with mock.patch('requests.get', return_value=test_response) as mock_request:
+        with mock.patch('requests.Session.get', return_value=test_response) as mock_request:
             project_config_manager = config_manager.PollingConfigManager(sdk_key=sdk_key, logger=mock_logger)
             project_config_manager.stop()
 
@@ -434,7 +434,7 @@ def raise_for_status(self):
         self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig)
 
         # Call fetch_datafile again, but raise exception this time
-        with mock.patch('requests.get', return_value=MockExceptionResponse()) as mock_requests:
+        with mock.patch('requests.Session.get', return_value=MockExceptionResponse()) as mock_requests:
             project_config_manager._initialize_thread()
             project_config_manager.start()
             project_config_manager.stop()
@@ -462,7 +462,7 @@ def test_fetch_datafile__request_exception_raised(self, _):
         test_response.status_code = 200
         test_response.headers = test_headers
         test_response._content = test_datafile
-        with mock.patch('requests.get', return_value=test_response) as mock_request:
+        with mock.patch('requests.Session.get', return_value=test_response) as mock_request:
             project_config_manager = config_manager.PollingConfigManager(sdk_key=sdk_key, logger=mock_logger)
             project_config_manager.stop()
 
@@ -476,7 +476,7 @@ def test_fetch_datafile__request_exception_raised(self, _):
 
         # Call fetch_datafile again, but raise exception this time
         with mock.patch(
-            'requests.get',
+            'requests.Session.get',
             side_effect=requests.exceptions.RequestException('Error Error !!'),
         ) as mock_requests:
             project_config_manager._initialize_thread()
@@ -506,7 +506,7 @@ def test_fetch_datafile__exception_polling_thread_failed(self, _):
         test_response.headers = test_headers
         test_response._content = test_datafile
 
-        with mock.patch('requests.get', return_value=test_response):
+        with mock.patch('requests.Session.get', return_value=test_response):
             project_config_manager = config_manager.PollingConfigManager(sdk_key=sdk_key,
                                                                          logger=mock_logger,
                                                                          update_interval=12345678912345)
@@ -516,8 +516,9 @@ def test_fetch_datafile__exception_polling_thread_failed(self, _):
             # verify the error log message
             log_messages = [args[0] for args, _ in mock_logger.error.call_args_list]
             for message in log_messages:
+                print(message)
                 if "Thread for background datafile polling failed. " \
-                   "Error: timestamp too large to convert to C _PyTime_t" not in message:
+                   "Error: timestamp too large to convert to C PyTime_t" not in message:
                     assert False
 
     def test_is_running(self, _):
@@ -529,7 +530,7 @@ def test_is_running(self, _):
         project_config_manager.stop()
 
 
-@mock.patch('requests.get')
+@mock.patch('requests.Session.get')
 class AuthDatafilePollingConfigManagerTest(base.BaseTest):
     def test_init__datafile_access_token_none__fails(self, _):
         """ Test that initialization fails if datafile_access_token is None. """
@@ -569,7 +570,7 @@ def test_fetch_datafile(self, _):
         test_response._content = test_datafile
 
         # Call fetch_datafile and assert that request was sent with correct authorization header
-        with mock.patch('requests.get',
+        with mock.patch('requests.Session.get',
                         return_value=test_response) as mock_request:
             project_config_manager.fetch_datafile()
 
@@ -596,7 +597,7 @@ def test_fetch_datafile__request_exception_raised(self, _):
         test_response._content = test_datafile
 
         # Call fetch_datafile and assert that request was sent with correct authorization header
-        with mock.patch('requests.get', return_value=test_response) as mock_request:
+        with mock.patch('requests.Session.get', return_value=test_response) as mock_request:
             project_config_manager = config_manager.AuthDatafilePollingConfigManager(
                 datafile_access_token=datafile_access_token,
                 sdk_key=sdk_key,
@@ -614,7 +615,7 @@ def test_fetch_datafile__request_exception_raised(self, _):
 
         # Call fetch_datafile again, but raise exception this time
         with mock.patch(
-            'requests.get',
+            'requests.Session.get',
             side_effect=requests.exceptions.RequestException('Error Error !!'),
         ) as mock_requests:
             project_config_manager._initialize_thread()

tests/test_event_dispatcher.py

@@ -29,7 +29,7 @@ def test_dispatch_event__get_request(self):
         params = {'a': '111001', 'n': 'test_event', 'g': '111028', 'u': 'oeutest_user'}
         event = event_builder.Event(url, params)
 
-        with mock.patch('requests.get') as mock_request_get:
+        with mock.patch('requests.Session.get') as mock_request_get:
             event_dispatcher.EventDispatcher.dispatch_event(event)
 
         mock_request_get.assert_called_once_with(url, params=params, timeout=EventDispatchConfig.REQUEST_TIMEOUT)
@@ -46,7 +46,7 @@ def test_dispatch_event__post_request(self):
         }
         event = event_builder.Event(url, params, http_verb='POST', headers={'Content-Type': 'application/json'})
 
-        with mock.patch('requests.post') as mock_request_post:
+        with mock.patch('requests.Session.post') as mock_request_post:
             event_dispatcher.EventDispatcher.dispatch_event(event)
 
         mock_request_post.assert_called_once_with(
@@ -69,7 +69,7 @@ def test_dispatch_event__handle_request_exception(self):
         event = event_builder.Event(url, params, http_verb='POST', headers={'Content-Type': 'application/json'})
 
         with mock.patch(
-            'requests.post', side_effect=request_exception.RequestException('Failed Request'),
+            'requests.Session.post', side_effect=request_exception.RequestException('Failed Request'),
         ) as mock_request_post, mock.patch('logging.error') as mock_log_error:
             event_dispatcher.EventDispatcher.dispatch_event(event)
 

tests/test_notification_center_registry.py

@@ -60,7 +60,7 @@ def test_remove_notification_center(self):
         test_response = self.fake_server_response(status_code=200, content=test_datafile)
         notification_center = _NotificationCenterRegistry.get_notification_center(sdk_key, logger)
 
-        with mock.patch('requests.get', return_value=test_response), \
+        with mock.patch('requests.Session.get', return_value=test_response), \
              mock.patch.object(notification_center, 'send_notifications') as mock_send:
 
             client = Optimizely(sdk_key=sdk_key, logger=logger)

tests/test_optimizely.py

@@ -4696,7 +4696,7 @@ def delay(*args, **kwargs):
             time.sleep(.5)
             return mock.DEFAULT
 
-        with mock.patch('requests.get', return_value=test_response, side_effect=delay):
+        with mock.patch('requests.Session.get', return_value=test_response, side_effect=delay):
             # initialize config_manager with delay, so it will receive the datafile after client initialization
             custom_config_manager = config_manager.PollingConfigManager(sdk_key='segments-test', logger=logger)
             client = optimizely.Optimizely(config_manager=custom_config_manager)
@@ -5428,7 +5428,7 @@ def test_send_odp_event__send_event_with_static_config_manager(self):
     def test_send_odp_event__send_event_with_polling_config_manager(self):
         mock_logger = mock.Mock()
         with mock.patch(
-            'requests.get',
+            'requests.Session.get',
             return_value=self.fake_server_response(
                 status_code=200,
                 content=json.dumps(self.config_dict_with_audience_segments)
@@ -5467,7 +5467,7 @@ def test_send_odp_event__log_debug_if_datafile_not_ready(self):
     def test_send_odp_event__log_error_if_odp_not_enabled_with_polling_config_manager(self):
         mock_logger = mock.Mock()
         with mock.patch(
-            'requests.get',
+            'requests.Session.get',
             return_value=self.fake_server_response(
                 status_code=200,
                 content=json.dumps(self.config_dict_with_audience_segments)

tests/test_optimizely_factory.py

@@ -26,7 +26,7 @@
 from . import base
 
 
-@mock.patch('requests.get')
+@mock.patch('requests.Session.get')
 class OptimizelyFactoryTest(base.BaseTest):
     def delay(*args, **kwargs):
         time.sleep(.5)
@@ -171,7 +171,7 @@ def test_set_batch_size_and_set_flush_interval___should_set_values_valid_or_inva
         self.assertEqual(optimizely_instance.event_processor.batch_size, 10)
 
     def test_update_odp_config_correctly(self, _):
-        with mock.patch('requests.get') as mock_request_post:
+        with mock.patch('requests.Session.get') as mock_request_post:
             mock_request_post.return_value = self.fake_server_response(
                 status_code=200,
                 content=json.dumps(self.config_dict_with_audience_segments)
@@ -194,7 +194,7 @@ def test_update_odp_config_correctly_with_custom_config_manager_and_delay(self,
         test_datafile = json.dumps(self.config_dict_with_audience_segments)
         test_response = self.fake_server_response(status_code=200, content=test_datafile)
 
-        with mock.patch('requests.get', return_value=test_response, side_effect=self.delay):
+        with mock.patch('requests.Session.get', return_value=test_response, side_effect=self.delay):
             # initialize config_manager with delay, so it will receive the datafile after client initialization
             config_manager = PollingConfigManager(sdk_key='test', logger=logger)
             client = OptimizelyFactory.default_instance_with_config_manager(config_manager=config_manager)
@@ -221,7 +221,7 @@ def test_update_odp_config_correctly_with_delay(self, _):
         test_datafile = json.dumps(self.config_dict_with_audience_segments)
         test_response = self.fake_server_response(status_code=200, content=test_datafile)
 
-        with mock.patch('requests.get', return_value=test_response, side_effect=self.delay):
+        with mock.patch('requests.Session.get', return_value=test_response, side_effect=self.delay):
             # initialize config_manager with delay, so it will receive the datafile after client initialization
             client = OptimizelyFactory.default_instance(sdk_key='test')
             odp_manager = client.odp_manager
@@ -247,7 +247,7 @@ def test_odp_updated_with_custom_instance(self, _):
         test_datafile = json.dumps(self.config_dict_with_audience_segments)
         test_response = self.fake_server_response(status_code=200, content=test_datafile)
 
-        with mock.patch('requests.get', return_value=test_response, side_effect=self.delay):
+        with mock.patch('requests.Session.get', return_value=test_response, side_effect=self.delay):
             # initialize config_manager with delay, so it will receive the datafile after client initialization
             client = OptimizelyFactory.custom_instance(sdk_key='test')
             odp_manager = client.odp_manager