From 3e4bf004bdeb52b901b103cbc4003750398c2403 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 12 Jan 2023 14:20:03 -0500 Subject: [PATCH 01/10] check if opti instance is valid on odp methods --- optimizely/optimizely.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 7a46f9276..04d5cfab0 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1352,9 +1352,17 @@ def _update_odp_config_on_datafile_update(self) -> None: ) def identify_user(self, user_id: str) -> None: + if not self.is_valid: + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('identify_user')) + return + self.odp_manager.identify_user(user_id) def fetch_qualified_segments(self, user_id: str, options: Optional[list[str]] = None) -> Optional[list[str]]: + if not self.is_valid: + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('fetch_qualified_segments')) + return None + return self.odp_manager.fetch_qualified_segments(user_id, options or []) def send_odp_event( @@ -1374,11 +1382,16 @@ def send_odp_event( data: An optional dictionary for associated data. The default event data will be added to this data before sending to the ODP server. """ + if not self.is_valid: + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('send_odp_event')) + return + self.odp_manager.send_event(type, action, identifiers or {}, data or {}) def close(self) -> None: if callable(getattr(self.event_processor, 'stop', None)): self.event_processor.stop() # type: ignore[attr-defined] - self.odp_manager.close() + if self.is_valid: + self.odp_manager.close() if callable(getattr(self.config_manager, 'stop', None)): self.config_manager.stop() # type: ignore[attr-defined] From 6f09271a3cdda8bd404419f59347210de8c35951 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 12 Jan 2023 14:25:26 -0500 Subject: [PATCH 02/10] fix variable missing error --- optimizely/optimizely_config.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 397ddba52..c4f55d862 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -343,9 +343,11 @@ def _get_variables_map( # set variation specific variable value if any if variation.get('featureEnabled'): + feature_variables_map = self.feature_key_variable_id_to_variable_map[feature_flag['key']] for variable in variation.get('variables', []): - feature_variable = self.feature_key_variable_id_to_variable_map[feature_flag['key']][variable['id']] - variables_map[feature_variable.key].value = variable['value'] + feature_variable = feature_variables_map.get(variable['id']) + if feature_variable: + variables_map[feature_variable.key].value = variable['value'] return variables_map From 657307d98335ad6f257a6a9c14a3faa7fc0db1e4 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Fri, 13 Jan 2023 10:25:06 -0500 Subject: [PATCH 03/10] fix extrenous identify calls --- optimizely/optimizely.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 04d5cfab0..297005bb8 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -343,7 +343,7 @@ def _get_feature_variable_for_type( source_info = {} variable_value = variable.defaultValue - user_context = self.create_user_context(user_id, attributes) + user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) # error is logged in create_user_context if user_context is None: return None @@ -432,7 +432,7 @@ def _get_all_feature_variables_for_type( feature_enabled = False source_info = {} - user_context = self.create_user_context(user_id, attributes) + user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) # error is logged in create_user_context if user_context is None: return None @@ -641,7 +641,7 @@ def get_variation( if not self._validate_user_inputs(attributes): return None - user_context = self.create_user_context(user_id, attributes) + user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) # error is logged in create_user_context if not user_context: return None @@ -703,7 +703,7 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona feature_enabled = False source_info = {} - user_context = self.create_user_context(user_id, attributes) + user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) # error is logged in create_user_context if not user_context: return False From b424ecb26957e032192be0b7164498685add0b94 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Fri, 13 Jan 2023 13:49:14 -0500 Subject: [PATCH 04/10] change integrations to default to first with key --- optimizely/entities.py | 2 +- optimizely/project_config.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/optimizely/entities.py b/optimizely/entities.py index 63b54f68a..fed1a49a7 100644 --- a/optimizely/entities.py +++ b/optimizely/entities.py @@ -188,7 +188,7 @@ def __str__(self) -> str: class Integration(BaseEntity): - def __init__(self, key: str, host: Optional[str] = None, publicKey: Optional[str] = None): + def __init__(self, key: str, host: Optional[str] = None, publicKey: Optional[str] = None, **kwargs: Any): self.key = key self.host = host self.publicKey = publicKey diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 9490e7356..adfeee415 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -112,7 +112,9 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any): self.experiment_id_map[experiment_dict['id']] = entities.Experiment(**experiment_dict) if self.integrations: - self.integration_key_map = self._generate_key_map(self.integrations, 'key', entities.Integration) + self.integration_key_map = self._generate_key_map( + self.integrations, 'key', entities.Integration, first_value=True + ) odp_integration = self.integration_key_map.get('odp') if odp_integration: self.public_key_for_odp = odp_integration.publicKey @@ -191,7 +193,7 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any): @staticmethod def _generate_key_map( - entity_list: Iterable[Any], key: str, entity_class: Type[EntityClass] + entity_list: Iterable[Any], key: str, entity_class: Type[EntityClass], first_value: bool = False ) -> dict[str, EntityClass]: """ Helper method to generate map from key to entity object for given list of dicts. @@ -199,13 +201,16 @@ def _generate_key_map( entity_list: List consisting of dict. key: Key in each dict which will be key in the map. entity_class: Class representing the entity. + first_value: If True, only save the first value found for each key. Returns: Map mapping key to entity object. """ - key_map = {} + key_map: dict[str, EntityClass] = {} for obj in entity_list: + if first_value and key_map.get(obj[key]): + continue key_map[obj[key]] = entity_class(**obj) return key_map From 1647fe9dbae8406536e9089b189a4b4977bca82b Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Fri, 13 Jan 2023 13:58:26 -0500 Subject: [PATCH 05/10] fix cache_size bug --- optimizely/optimizely.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 297005bb8..33cee1a30 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1328,8 +1328,8 @@ def setup_odp(self) -> None: if not self.sdk_settings.segments_cache: self.sdk_settings.segments_cache = LRUCache( - self.sdk_settings.segments_cache_size or enums.OdpSegmentsCacheConfig.DEFAULT_CAPACITY, - self.sdk_settings.segments_cache_timeout_in_secs or enums.OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS + self.sdk_settings.segments_cache_size, + self.sdk_settings.segments_cache_timeout_in_secs ) def _update_odp_config_on_datafile_update(self) -> None: From 40bc9d2b448a1192bd560767f02e73ff9bc9a88a Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 18 Jan 2023 18:02:06 -0500 Subject: [PATCH 06/10] add timeout to pollingconfig stop --- optimizely/config_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index c5cf8bca5..9d26fa3a0 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -375,10 +375,11 @@ def is_running(self) -> bool: return self._polling_thread.is_alive() def stop(self) -> None: - """ Stop the polling thread and wait for it to exit. """ + """ Stop the polling thread and briefly wait for it to exit. """ if self.is_running: self.stopped.set() - self._polling_thread.join() + # no need to wait too long as this exists to avoid interfering with tests + self._polling_thread.join(timeout=0.2) def _run(self) -> None: """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ From 4199cc595f604a7a6b378ef2eff3e9cc61f775b1 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 19 Jan 2023 14:51:17 -0800 Subject: [PATCH 07/10] Update python.yml --- .github/workflows/python.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 2df01f723..38d45e9c0 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -5,9 +5,9 @@ name: build on: push: - branches: [ master ] + branches: [ aleap/odp_fixes ] pull_request: - branches: [ master ] + branches: [ aleap/odp_fixes ] jobs: lint_markdown_files: From e36772d3409fe20d2eee84114e04221b9d39f923 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 19 Jan 2023 15:29:24 -0800 Subject: [PATCH 08/10] revert branch to master --- .github/workflows/python.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 38d45e9c0..7cf833626 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -5,9 +5,9 @@ name: build on: push: - branches: [ aleap/odp_fixes ] + branches: [ master ] pull_request: - branches: [ aleap/odp_fixes ] + branches: [ master ] jobs: lint_markdown_files: From 4877695e8e6446d5b92f02fecf34a9642f997f16 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 19 Jan 2023 19:25:59 -0500 Subject: [PATCH 09/10] fix create_user_context --- optimizely/optimizely.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 5f4ecc35c..df469471a 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1083,7 +1083,7 @@ def create_user_context( self.logger.error(enums.Errors.INVALID_INPUT.format('attributes')) return None - return OptimizelyUserContext(self, self.logger, user_id, attributes) + return OptimizelyUserContext(self, self.logger, user_id, attributes, True) def _decide( self, user_context: Optional[OptimizelyUserContext], key: str, From 53630640765dc31e19ff4e25ca5f9132f5f1554c Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Tue, 24 Jan 2023 10:18:43 -0500 Subject: [PATCH 10/10] remove unnecessary checks --- optimizely/optimizely.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index df469471a..8408cbcc9 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -346,9 +346,7 @@ def _get_feature_variable_for_type( variable_value = variable.defaultValue user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) - # error is logged in create_user_context - if user_context is None: - return None + decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context) if decision.variation: @@ -435,9 +433,7 @@ def _get_all_feature_variables_for_type( source_info = {} user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) - # error is logged in create_user_context - if user_context is None: - return None + decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context) if decision.variation: @@ -644,9 +640,6 @@ def get_variation( return None user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) - # error is logged in create_user_context - if not user_context: - return None variation, _ = self.decision_service.get_variation(project_config, experiment, user_context) if variation: @@ -705,10 +698,8 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona feature_enabled = False source_info = {} + user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) - # error is logged in create_user_context - if not user_context: - return False decision, _ = self.decision_service.get_variation_for_feature(project_config, feature, user_context) is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST