Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Feb 26, 2025
34 changes: 28 additions & 6 deletions optimizely/config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 15 additions & 3 deletions optimizely/event_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ class VersionType:
class EventDispatchConfig:
"""Event dispatching configs."""
REQUEST_TIMEOUT: Final = 10
RETRIES: Final = 3


class OdpEventApiConfig:
Expand Down
5 changes: 3 additions & 2 deletions optimizely/helpers/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 14 additions & 13 deletions tests/test_config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. """
Expand Down Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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, _):
Expand All @@ -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. """
Expand Down Expand Up @@ -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()

Expand All @@ -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,
Expand All @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions tests/test_event_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_notification_center_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions tests/test_optimizely_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down