From a0bdd1cc63d3eaec439c8932d24f33000eb525f5 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 6 Oct 2023 00:07:31 -0700 Subject: [PATCH 1/3] add try except to all public client methods (FF-947) --- eppo_client/__init__.py | 2 + eppo_client/client.py | 137 ++++++++++++++++++++++++++-------------- eppo_client/config.py | 1 + test/client_test.py | 31 +++++++++ 4 files changed, 123 insertions(+), 48 deletions(-) diff --git a/eppo_client/__init__.py b/eppo_client/__init__.py index 575524d..9caa004 100644 --- a/eppo_client/__init__.py +++ b/eppo_client/__init__.py @@ -38,6 +38,7 @@ def init(config: Config) -> EppoClient: http_client=http_client, config_store=config_store ) assignment_logger = config.assignment_logger + is_graceful_mode = config.is_graceful_mode global __client global __lock try: @@ -48,6 +49,7 @@ def init(config: Config) -> EppoClient: __client = EppoClient( config_requestor=config_requestor, assignment_logger=assignment_logger, + is_graceful_mode=is_graceful_mode, ) return __client finally: diff --git a/eppo_client/client.py b/eppo_client/client.py index b043b6f..1f65be3 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -25,9 +25,11 @@ def __init__( self, config_requestor: ExperimentConfigurationRequestor, assignment_logger: AssignmentLogger, + is_graceful_mode: bool = True, ): self.__config_requestor = config_requestor self.__assignment_logger = assignment_logger + self.__is_graceful_mode = is_graceful_mode self.__poller = Poller( interval_millis=POLL_INTERVAL_MILLIS, jitter_millis=POLL_JITTER_MILLIS, @@ -38,62 +40,92 @@ def __init__( def get_string_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() ) -> Optional[str]: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.STRING - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) + try: + assigned_variation = self.get_assignment_variation( + subject_key, flag_key, subject_attributes, VariationType.STRING + ) + return ( + assigned_variation.typed_value + if assigned_variation is not None + else assigned_variation + ) + except Exception as e: + if self.__is_graceful_mode: + logger.error("[Eppo SDK] Error getting assignment: " + str(e)) + return None + raise e def get_numeric_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() ) -> Optional[Number]: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.NUMERIC - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) + try: + assigned_variation = self.get_assignment_variation( + subject_key, flag_key, subject_attributes, VariationType.NUMERIC + ) + return ( + assigned_variation.typed_value + if assigned_variation is not None + else assigned_variation + ) + except Exception as e: + if self.__is_graceful_mode: + logger.error("[Eppo SDK] Error getting assignment: " + str(e)) + return None + raise e def get_boolean_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() ) -> Optional[bool]: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.BOOLEAN - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) + try: + assigned_variation = self.get_assignment_variation( + subject_key, flag_key, subject_attributes, VariationType.BOOLEAN + ) + return ( + assigned_variation.typed_value + if assigned_variation is not None + else assigned_variation + ) + except Exception as e: + if self.__is_graceful_mode: + logger.error("[Eppo SDK] Error getting assignment: " + str(e)) + return None + raise e def get_parsed_json_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() ) -> Optional[Dict[Any, Any]]: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.JSON - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) + try: + assigned_variation = self.get_assignment_variation( + subject_key, flag_key, subject_attributes, VariationType.JSON + ) + return ( + assigned_variation.typed_value + if assigned_variation is not None + else assigned_variation + ) + except Exception as e: + if self.__is_graceful_mode: + logger.error("[Eppo SDK] Error getting assignment: " + str(e)) + return None + raise e def get_json_string_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() ) -> Optional[str]: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.JSON - ) - return ( - assigned_variation.value - if assigned_variation is not None - else assigned_variation - ) + try: + assigned_variation = self.get_assignment_variation( + subject_key, flag_key, subject_attributes, VariationType.JSON + ) + return ( + assigned_variation.value + if assigned_variation is not None + else assigned_variation + ) + except Exception as e: + if self.__is_graceful_mode: + logger.error("[Eppo SDK] Error getting assignment: " + str(e)) + return None + raise e @deprecated( "get_assignment is deprecated in favor of the typed get__assignment methods" @@ -101,14 +133,23 @@ def get_json_string_assignment( def get_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() ) -> Optional[str]: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes - ) - return ( - assigned_variation.value - if assigned_variation is not None - else assigned_variation - ) + try: + assigned_variation = self.get_assignment_variation( + subject_key, flag_key, subject_attributes + ) + return ( + assigned_variation.value + if assigned_variation is not None + else assigned_variation + ) + except ValueError as e: + # allow ValueError to bubble up as it is a validation error + raise e + except Exception as e: + if self.__is_graceful_mode: + logger.error("[Eppo SDK] Error getting assignment: " + str(e)) + return None + raise e def get_assignment_variation( self, diff --git a/eppo_client/config.py b/eppo_client/config.py index 404897b..d9d7d88 100644 --- a/eppo_client/config.py +++ b/eppo_client/config.py @@ -8,6 +8,7 @@ class Config(SdkBaseModel): api_key: str base_url: str = "https://fscdn.eppo.cloud/api" assignment_logger: AssignmentLogger + is_graceful_mode: bool = True def _validate(self): validate_not_blank("api_key", self.api_key) diff --git a/test/client_test.py b/test/client_test.py index 11deb15..7b0a02a 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -252,6 +252,37 @@ def test_with_null_experiment_config(mock_config_requestor): assert client.get_assignment("user-1", "experiment-key-1") is None +@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") +@patch.object(EppoClient, "get_assignment_variation") +def test_graceful_mode_on(mock_get_assignment_variation, mock_config_requestor): + mock_get_assignment_variation.side_effect = Exception("This is a mock exception!") + + client = EppoClient( + config_requestor=mock_config_requestor, + assignment_logger=AssignmentLogger(), + is_graceful_mode=True, + ) + + res = client.get_assignment("user-1", "experiment-key-1") + mock_get_assignment_variation.assert_called_once() + assert res is None + + +@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") +@patch.object(EppoClient, "get_assignment_variation") +def test_graceful_mode_off(mock_get_assignment_variation, mock_config_requestor): + mock_get_assignment_variation.side_effect = Exception("This is a mock exception!") + + client = EppoClient( + config_requestor=mock_config_requestor, + assignment_logger=AssignmentLogger(), + is_graceful_mode=False, + ) + + with pytest.raises(Exception) as exc_info: + client.get_assignment("user-1", "experiment-key-1") + + @pytest.mark.parametrize("test_case", test_data) def test_assign_subject_in_sample(test_case): print("---- Test case for {} Experiment".format(test_case["experiment"])) From 1fc5cb7fa310029e8211b890ed8e92d015ff423c Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 6 Oct 2023 00:11:28 -0700 Subject: [PATCH 2/3] test all methods --- test/client_test.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/client_test.py b/test/client_test.py index 7b0a02a..36d970e 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -263,9 +263,12 @@ def test_graceful_mode_on(mock_get_assignment_variation, mock_config_requestor): is_graceful_mode=True, ) - res = client.get_assignment("user-1", "experiment-key-1") - mock_get_assignment_variation.assert_called_once() - assert res is None + assert client.get_assignment("user-1", "experiment-key-1") is None + assert client.get_boolean_assignment("user-1", "experiment-key-1") is None + assert client.get_json_string_assignment("user-1", "experiment-key-1") is None + assert client.get_numeric_assignment("user-1", "experiment-key-1") is None + assert client.get_string_assignment("user-1", "experiment-key-1") is None + assert client.get_parsed_json_assignment("user-1", "experiment-key-1") is None @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") @@ -279,8 +282,13 @@ def test_graceful_mode_off(mock_get_assignment_variation, mock_config_requestor) is_graceful_mode=False, ) - with pytest.raises(Exception) as exc_info: + with pytest.raises(Exception): client.get_assignment("user-1", "experiment-key-1") + client.get_boolean_assignment("user-1", "experiment-key-1") + client.get_json_string_assignment("user-1", "experiment-key-1") + client.get_numeric_assignment("user-1", "experiment-key-1") + client.get_string_assignment("user-1", "experiment-key-1") + client.get_parsed_json_assignment("user-1", "experiment-key-1") @pytest.mark.parametrize("test_case", test_data) From 024cd5ef71e2525e010008fdb4f451173d591c84 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 6 Oct 2023 11:12:19 -0700 Subject: [PATCH 3/3] value error in all public methods --- eppo_client/client.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/eppo_client/client.py b/eppo_client/client.py index 1f65be3..1db6ddf 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -49,6 +49,9 @@ def get_string_assignment( if assigned_variation is not None else assigned_variation ) + except ValueError as e: + # allow ValueError to bubble up as it is a validation error + raise e except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) @@ -67,6 +70,9 @@ def get_numeric_assignment( if assigned_variation is not None else assigned_variation ) + except ValueError as e: + # allow ValueError to bubble up as it is a validation error + raise e except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) @@ -85,6 +91,9 @@ def get_boolean_assignment( if assigned_variation is not None else assigned_variation ) + except ValueError as e: + # allow ValueError to bubble up as it is a validation error + raise e except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) @@ -103,6 +112,9 @@ def get_parsed_json_assignment( if assigned_variation is not None else assigned_variation ) + except ValueError as e: + # allow ValueError to bubble up as it is a validation error + raise e except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) @@ -121,6 +133,9 @@ def get_json_string_assignment( if assigned_variation is not None else assigned_variation ) + except ValueError as e: + # allow ValueError to bubble up as it is a validation error + raise e except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e))