diff --git a/.gitignore b/.gitignore index cff402c4..00ad86a4 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ MANIFEST .idea/* .*virtualenv/* .mypy_cache +.vscode/* # Output of building package *.egg-info @@ -26,3 +27,4 @@ datafile.json # Sphinx documentation docs/build/ + diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f3bc3cb..d0cd8b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Optimizely Python SDK Changelog +## 5.2.0 +February 26, 2025 + +Python threads have been named. + +`PollingConfigManager` now has another optional parameter `retries` that will control how many times the SDK will attempt to get the datafile if the connection fails. Previously, the SDK would only try once. Now it defaults to maximum of three attempts. When sending event data, the SDK will attempt to send event data up to three times, where as before it would only attempt once. + ## 5.1.0 November 27th, 2024 diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 755c6b9c..3dce2741 100644 --- a/optimizely/config_manager.py +++ b/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 @@ -432,7 +445,7 @@ def start(self) -> None: self._polling_thread.start() def _initialize_thread(self) -> None: - self._polling_thread = threading.Thread(target=self._run, daemon=True) + self._polling_thread = threading.Thread(target=self._run, name="PollThread", daemon=True) class AuthDatafilePollingConfigManager(PollingConfigManager): @@ -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 diff --git a/optimizely/event/event_processor.py b/optimizely/event/event_processor.py index 9445ffc6..05f5e078 100644 --- a/optimizely/event/event_processor.py +++ b/optimizely/event/event_processor.py @@ -186,8 +186,7 @@ def start(self) -> None: return self.flushing_interval_deadline = self._get_time() + self._get_time(self.flush_interval.total_seconds()) - self.executor = threading.Thread(target=self._run) - self.executor.daemon = True + self.executor = threading.Thread(target=self._run, name="EventThread", daemon=True) self.executor.start() def _run(self) -> None: diff --git a/optimizely/event_dispatcher.py b/optimizely/event_dispatcher.py index e2ca54f0..767fbb7d 100644 --- a/optimizely/event_dispatcher.py +++ b/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() diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 1c7a8e1c..fe90946e 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -198,6 +198,7 @@ class VersionType: class EventDispatchConfig: """Event dispatching configs.""" REQUEST_TIMEOUT: Final = 10 + RETRIES: Final = 3 class OdpEventApiConfig: diff --git a/optimizely/helpers/validator.py b/optimizely/helpers/validator.py index 17cff87c..b9e4fcc5 100644 --- a/optimizely/helpers/validator.py +++ b/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 diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index 18b08eb0..85512e90 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -75,7 +75,7 @@ def __init__( self.retry_count = OdpEventManagerConfig.DEFAULT_RETRY_COUNT self._current_batch: list[OdpEvent] = [] """_current_batch should only be modified by the processing thread, as it is not thread safe""" - self.thread = Thread(target=self._run, daemon=True) + self.thread = Thread(target=self._run, name="OdpThread", daemon=True) self.thread_exception = False """thread_exception will be True if the processing thread did not exit cleanly""" diff --git a/optimizely/optimizely_user_context.py b/optimizely/optimizely_user_context.py index fb674f93..e88c0f52 100644 --- a/optimizely/optimizely_user_context.py +++ b/optimizely/optimizely_user_context.py @@ -336,7 +336,7 @@ def _fetch_qualified_segments() -> bool: return success if callback: - fetch_thread = threading.Thread(target=_fetch_qualified_segments) + fetch_thread = threading.Thread(target=_fetch_qualified_segments, name="FetchQualifiedSegmentsThread") fetch_thread.start() return fetch_thread else: diff --git a/optimizely/version.py b/optimizely/version.py index 941e5e68..4f0f20c6 100644 --- a/optimizely/version.py +++ b/optimizely/version.py @@ -11,5 +11,5 @@ # See the License for the specific language governing permissions and # limitations under the License. -version_info = (5, 1, 0) +version_info = (5, 2, 0) __version__ = '.'.join(str(v) for v in version_info) diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 1c3fbe89..56674381 100644 --- a/tests/test_config_manager.py +++ b/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() diff --git a/tests/test_event_dispatcher.py b/tests/test_event_dispatcher.py index 7e075f47..30311e35 100644 --- a/tests/test_event_dispatcher.py +++ b/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) diff --git a/tests/test_notification_center_registry.py b/tests/test_notification_center_registry.py index 0f800cfd..81984059 100644 --- a/tests/test_notification_center_registry.py +++ b/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) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 8d36b830..1f4293cd 100644 --- a/tests/test_optimizely.py +++ b/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) diff --git a/tests/test_optimizely_factory.py b/tests/test_optimizely_factory.py index be41755a..989d960c 100644 --- a/tests/test_optimizely_factory.py +++ b/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