From 97f81a30a2ccc7a3623a0824229ecc67cad6c12b Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 10:59:37 -0700 Subject: [PATCH 01/23] Feature Management small fixes --- featuremanagement/_featuremanagerbase.py | 96 ++++++++++--------- .../azuremonitor/_send_telemetry.py | 72 +++++++------- 2 files changed, 89 insertions(+), 79 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index 64732e5..6816ec3 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -163,31 +163,36 @@ def _assign_variant( variant_name = None if not feature or not feature.variants or not feature.allocation: return - if feature.allocation.user and targeting_context.user_id: - for user_allocation in feature.allocation.user: + + allocation = feature.allocation + groups = targeting_context.groups + + if allocation.user and targeting_context.user_id: + for user_allocation in allocation.user: if targeting_context.user_id in user_allocation.users: evaluation_event.reason = VariantAssignmentReason.USER variant_name = user_allocation.variant - if not variant_name and feature.allocation.group and len(targeting_context.groups) > 0: - for group_allocation in feature.allocation.group: - for group in targeting_context.groups: - if group in group_allocation.groups: - evaluation_event.reason = VariantAssignmentReason.GROUP - variant_name = group_allocation.variant - if not variant_name and feature.allocation.percentile: - seed = feature.allocation.seed - if not seed: - seed = "allocation\n" + feature.name - context_id = targeting_context.user_id + "\n" + seed + + if not variant_name and allocation.group and groups: + for group_allocation in allocation.group: + if any(group in group_allocation.groups for group in groups): + evaluation_event.reason = VariantAssignmentReason.GROUP + variant_name = group_allocation.variant + + if not variant_name and allocation.percentile: + seed = allocation.seed or f"allocation\n{feature.name}" + context_id = f"{targeting_context.user_id}\n{seed}" box: float = self._is_targeted(context_id) - for percentile_allocation in feature.allocation.percentile: - if box == 100 and percentile_allocation.percentile_to == 100: - variant_name = percentile_allocation.variant - if not percentile_allocation.percentile_to: - continue - if percentile_allocation.percentile_from <= box < percentile_allocation.percentile_to: + for percentile_allocation in allocation.percentile: + percentile_to: Optional[int] = percentile_allocation.percentile_to + if percentile_to is not None and ( + (box == 100 and percentile_to == 100) + or (percentile_allocation.percentile_from <= box < percentile_to) + ): evaluation_event.reason = VariantAssignmentReason.PERCENTILE variant_name = percentile_allocation.variant + break + if not variant_name: FeatureManagerBase._check_default_enabled_variant(evaluation_event) if feature_flag.allocation: @@ -195,10 +200,10 @@ def _assign_variant( feature_flag, feature_flag.allocation.default_when_enabled ) return + evaluation_event.variant = self._variant_name_to_variant(feature_flag, variant_name) - if not feature_flag.variants: - return - FeatureManagerBase._check_variant_override(feature_flag.variants, variant_name, True, evaluation_event) + if feature_flag.variants: + FeatureManagerBase._check_variant_override(feature_flag.variants, variant_name, True, evaluation_event) def _variant_name_to_variant(self, feature_flag: FeatureFlag, variant_name: Optional[str]) -> Optional[Variant]: """ @@ -208,10 +213,9 @@ def _variant_name_to_variant(self, feature_flag: FeatureFlag, variant_name: Opti :param str variant_name: Name of the variant. :return: Variant object. """ - if not feature_flag.variants: - return None - if not variant_name: + if not feature_flag.variants or not variant_name: return None + for variant_reference in feature_flag.variants: if variant_reference.name == variant_name: return Variant(variant_reference.name, variant_reference.configuration_value) @@ -225,31 +229,37 @@ def _build_targeting_context(self, args: Tuple[Any]) -> TargetingContext: :param args: Arguments to build the TargetingContext. :return: TargetingContext """ - if len(args) == 1 and isinstance(args[0], str): - return TargetingContext(user_id=args[0], groups=[]) - if len(args) == 1 and isinstance(args[0], TargetingContext): - return args[0] + if len(args) == 1: + arg = args[0] + if isinstance(arg, str): + return TargetingContext(user_id=arg, groups=[]) + if isinstance(arg, TargetingContext): + return arg return TargetingContext() def _assign_allocation(self, evaluation_event: EvaluationEvent, targeting_context: TargetingContext) -> None: feature_flag = evaluation_event.feature if not feature_flag: return - if feature_flag.variants: - if not feature_flag.allocation: - if evaluation_event.enabled: - evaluation_event.reason = VariantAssignmentReason.DEFAULT_WHEN_ENABLED - return - evaluation_event.reason = VariantAssignmentReason.DEFAULT_WHEN_DISABLED - return - if not evaluation_event.enabled: - FeatureManagerBase._check_default_disabled_variant(evaluation_event) - evaluation_event.variant = self._variant_name_to_variant( - feature_flag, feature_flag.allocation.default_when_disabled - ) - return - self._assign_variant(feature_flag, targeting_context, evaluation_event) + if not feature_flag.variants: + return + + if not feature_flag.allocation: + evaluation_event.reason = ( + VariantAssignmentReason.DEFAULT_WHEN_ENABLED + if evaluation_event.enabled + else VariantAssignmentReason.DEFAULT_WHEN_DISABLED + ) + return + if not evaluation_event.enabled: + FeatureManagerBase._check_default_disabled_variant(evaluation_event) + evaluation_event.variant = self._variant_name_to_variant( + feature_flag, feature_flag.allocation.default_when_disabled + ) + return + + self._assign_variant(feature_flag, targeting_context, evaluation_event) def list_feature_flag_names(self) -> List[str]: """ diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index bd84fd7..b678c26 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -38,10 +38,12 @@ def track_event(event_name: str, user: str, event_properties: Optional[Dict[str, """ if not HAS_AZURE_MONITOR_EVENTS_EXTENSION: return - if event_properties is None: - event_properties = {} + + event_properties = event_properties or {} + if user: event_properties[TARGETING_ID] = user + azure_monitor_track_event(event_name, event_properties) @@ -53,50 +55,48 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: """ if not HAS_AZURE_MONITOR_EVENTS_EXTENSION: return - event: Dict[str, Optional[str]] = {} - if not evaluation_event.feature: + + feature = evaluation_event.feature + + if not feature: return - event[FEATURE_NAME] = evaluation_event.feature.name - event[ENABLED] = str(evaluation_event.enabled) - event["Version"] = EVALUATION_EVENT_VERSION + + event: Dict[str, Optional[str]] = { + FEATURE_NAME: feature.name, + ENABLED: str(evaluation_event.enabled), + "Version": EVALUATION_EVENT_VERSION, + } + + reason = evaluation_event.reason + variant = evaluation_event.variant # VariantAllocationPercentage - if evaluation_event.reason and evaluation_event.reason != VariantAssignmentReason.NONE: - if evaluation_event.variant: - event[VARIANT] = evaluation_event.variant.name - event[REASON] = evaluation_event.reason.value + if reason and reason != VariantAssignmentReason.NONE: + if variant: + event[VARIANT] = variant.name + event[REASON] = reason.value - if evaluation_event.reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED: + if reason in {VariantAssignmentReason.DEFAULT_WHEN_ENABLED, VariantAssignmentReason.PERCENTILE}: allocation_percentage = 0 + allocation = feature.allocation - if evaluation_event.feature.allocation and evaluation_event.feature.allocation.percentile: - for allocation in evaluation_event.feature.allocation.percentile: - if ( - evaluation_event.variant - and allocation.variant == evaluation_event.variant.name - and allocation.percentile_to - ): - allocation_percentage += allocation.percentile_to - allocation.percentile_from - - event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) - elif evaluation_event.reason == VariantAssignmentReason.PERCENTILE: - if evaluation_event.feature.allocation and evaluation_event.feature.allocation.percentile: - allocation_percentage = 0 - for allocation in evaluation_event.feature.allocation.percentile: - if ( - evaluation_event.variant - and allocation.variant == evaluation_event.variant.name - and allocation.percentile_to - ): - allocation_percentage += allocation.percentile_to - allocation.percentile_from + if allocation and allocation.percentile: + for alloc in allocation.percentile: + if variant and alloc.variant == variant.name and alloc.percentile_to: + allocation_percentage += alloc.percentile_to - alloc.percentile_from + + if reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED: + event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) + else: event["VariantAssignmentPercentage"] = str(allocation_percentage) # DefaultWhenEnabled - if evaluation_event.feature.allocation and evaluation_event.feature.allocation.default_when_enabled: - event["DefaultWhenEnabled"] = evaluation_event.feature.allocation.default_when_enabled + if feature.allocation and feature.allocation.default_when_enabled: + event["DefaultWhenEnabled"] = feature.allocation.default_when_enabled - if evaluation_event.feature.telemetry: - for metadata_key, metadata_value in evaluation_event.feature.telemetry.metadata.items(): + if feature.telemetry: + for metadata_key, metadata_value in feature.telemetry.metadata.items(): if metadata_key not in event: event[metadata_key] = metadata_value + track_event(EVENT_NAME, evaluation_event.user, event_properties=event) From 716b51566c1b83723023ddb603d606b82a20bd15 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 11:44:35 -0700 Subject: [PATCH 02/23] added tests --- .../azuremonitor/_send_telemetry.py | 29 ++--- tests/test_send_telemetry_appinsights.py | 102 +++++++++++++++++- 2 files changed, 118 insertions(+), 13 deletions(-) diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index b678c26..f0941fb 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -76,18 +76,23 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: event[VARIANT] = variant.name event[REASON] = reason.value - if reason in {VariantAssignmentReason.DEFAULT_WHEN_ENABLED, VariantAssignmentReason.PERCENTILE}: - allocation_percentage = 0 - allocation = feature.allocation - - if allocation and allocation.percentile: - for alloc in allocation.percentile: - if variant and alloc.variant == variant.name and alloc.percentile_to: - allocation_percentage += alloc.percentile_to - alloc.percentile_from - - if reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED: - event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) - else: + allocation_percentage = 0 + if ( + reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED + and feature.allocation + and feature.allocation.percentile + ): + for allocation in feature.allocation.percentile: + if variant and allocation.variant != variant.name and allocation.percentile_to: + allocation_percentage += allocation.percentile_to - allocation.percentile_from + + event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) + elif reason == VariantAssignmentReason.PERCENTILE: + if feature.allocation and feature.allocation.percentile: + + for allocation in feature.allocation.percentile: + if variant and allocation.variant == variant.name and allocation.percentile_to: + allocation_percentage += allocation.percentile_to - allocation.percentile_from event["VariantAssignmentPercentage"] = str(allocation_percentage) # DefaultWhenEnabled diff --git a/tests/test_send_telemetry_appinsights.py b/tests/test_send_telemetry_appinsights.py index 96ebbbb..636ee27 100644 --- a/tests/test_send_telemetry_appinsights.py +++ b/tests/test_send_telemetry_appinsights.py @@ -13,7 +13,20 @@ class TestSendTelemetryAppinsights: def test_send_telemetry_appinsights(self): - feature_flag = FeatureFlag.convert_from_json({"id": "TestFeature"}) + feature_flag = FeatureFlag.convert_from_json( + { + "id": "TestFeature", + "telemetry": { + "enabled": True, + "metadata": { + "ETag": "cmwBRcIAq1jUyKL3Kj8bvf9jtxBrFg-R-ayExStMC90", + "FeatureFlagReference": "fake-store-uri/kv/.appconfig.featureflag/TestFeature", + "FeatureFlagId": "7vpkRJe452WVvlKXfA5XF3ASllwKsYZfC7D4w05rIoo", + "AllocationId": "MExY1waco2tqen4EcJKK", + }, + }, + } + ) evaluation_event = EvaluationEvent(feature_flag) variant = Variant("TestVariant", None) evaluation_event.feature = feature_flag @@ -34,6 +47,14 @@ def test_send_telemetry_appinsights(self): assert mock_track_event.call_args[0][1]["TargetingId"] == "test_user" assert mock_track_event.call_args[0][1]["Variant"] == "TestVariant" assert mock_track_event.call_args[0][1]["VariantAssignmentReason"] == "DefaultWhenDisabled" + assert mock_track_event.call_args[0][1]["ETag"] == "cmwBRcIAq1jUyKL3Kj8bvf9jtxBrFg-R-ayExStMC90" + assert ( + mock_track_event.call_args[0][1]["FeatureFlagReference"] + == "fake-store-uri/kv/.appconfig.featureflag/TestFeature" + ) + assert mock_track_event.call_args[0][1]["FeatureFlagId"] == "7vpkRJe452WVvlKXfA5XF3ASllwKsYZfC7D4w05rIoo" + assert mock_track_event.call_args[0][1]["AllocationId"] == "MExY1waco2tqen4EcJKK" + assert "DefaultWhenEnabled" not in mock_track_event.call_args[0][1] def test_send_telemetry_appinsights_no_user(self): feature_flag = FeatureFlag.convert_from_json({"id": "TestFeature"}) @@ -56,6 +77,7 @@ def test_send_telemetry_appinsights_no_user(self): assert "TargetingId" not in mock_track_event.call_args[0][1] assert mock_track_event.call_args[0][1]["Variant"] == "TestVariant" assert mock_track_event.call_args[0][1]["VariantAssignmentReason"] == "DefaultWhenDisabled" + assert "DefaultWhenEnabled" not in mock_track_event.call_args[0][1] def test_send_telemetry_appinsights_no_variant(self): feature_flag = FeatureFlag.convert_from_json({"id": "TestFeature"}) @@ -76,3 +98,81 @@ def test_send_telemetry_appinsights_no_variant(self): assert mock_track_event.call_args[0][1]["TargetingId"] == "test_user" assert "Variant" not in mock_track_event.call_args[0][1] assert "Reason" not in mock_track_event.call_args[0][1] + + def test_send_telemetry_appinsights_no_feature_flag(self): + evaluation_event = EvaluationEvent(None) + evaluation_event.enabled = True + evaluation_event.user = "test_user" + + with patch("featuremanagement.azuremonitor._send_telemetry.azure_monitor_track_event") as mock_track_event: + # This is called like this so we can override the track_event function + featuremanagement.azuremonitor._send_telemetry.publish_telemetry( # pylint: disable=protected-access + evaluation_event + ) + mock_track_event.assert_not_called() + + def test_send_telemetry_appinsights_default_when_enabled(self): + feature_flag = FeatureFlag.convert_from_json( + { + "id": "TestFeature", + "allocation": { + "default_when_enabled": "big", + "percentile": [{"from": 0, "to": 25, "variant": "big"}, {"from": 25, "to": 75, "variant": "small"}], + }, + } + ) + evaluation_event = EvaluationEvent(feature_flag) + variant = Variant("big", None) + evaluation_event.feature = feature_flag + evaluation_event.enabled = True + evaluation_event.user = "test_user" + evaluation_event.variant = variant + evaluation_event.reason = VariantAssignmentReason.DEFAULT_WHEN_ENABLED + + with patch("featuremanagement.azuremonitor._send_telemetry.azure_monitor_track_event") as mock_track_event: + # This is called like this so we can override the track_event function + featuremanagement.azuremonitor._send_telemetry.publish_telemetry( # pylint: disable=protected-access + evaluation_event + ) + mock_track_event.assert_called_once() + assert mock_track_event.call_args[0][0] == "FeatureEvaluation" + assert mock_track_event.call_args[0][1]["FeatureName"] == "TestFeature" + assert mock_track_event.call_args[0][1]["Enabled"] == "True" + assert mock_track_event.call_args[0][1]["TargetingId"] == "test_user" + assert mock_track_event.call_args[0][1]["Variant"] == "big" + assert mock_track_event.call_args[0][1]["VariantAssignmentReason"] == "DefaultWhenEnabled" + assert mock_track_event.call_args[0][1]["VariantAssignmentPercentage"] == "50" + assert "DefaultWhenEnabled" in mock_track_event.call_args[0][1] + assert mock_track_event.call_args[0][1]["DefaultWhenEnabled"] == "big" + + def test_send_telemetry_appinsights_allocation(self): + feature_flag = FeatureFlag.convert_from_json( + { + "id": "TestFeature", + "allocation": { + "percentile": [{"from": 0, "to": 25, "variant": "big"}, {"from": 25, "to": 75, "variant": "small"}] + }, + } + ) + evaluation_event = EvaluationEvent(feature_flag) + variant = Variant("big", None) + evaluation_event.feature = feature_flag + evaluation_event.enabled = True + evaluation_event.user = "test_user" + evaluation_event.variant = variant + evaluation_event.reason = VariantAssignmentReason.PERCENTILE + + with patch("featuremanagement.azuremonitor._send_telemetry.azure_monitor_track_event") as mock_track_event: + # This is called like this so we can override the track_event function + featuremanagement.azuremonitor._send_telemetry.publish_telemetry( # pylint: disable=protected-access + evaluation_event + ) + mock_track_event.assert_called_once() + assert mock_track_event.call_args[0][0] == "FeatureEvaluation" + assert mock_track_event.call_args[0][1]["FeatureName"] == "TestFeature" + assert mock_track_event.call_args[0][1]["Enabled"] == "True" + assert mock_track_event.call_args[0][1]["TargetingId"] == "test_user" + assert mock_track_event.call_args[0][1]["Variant"] == "big" + assert mock_track_event.call_args[0][1]["VariantAssignmentReason"] == "Percentile" + assert mock_track_event.call_args[0][1]["VariantAssignmentPercentage"] == "25" + assert "DefaultWhenEnabled" not in mock_track_event.call_args[0][1] From 2db5057bef8ba6077c55848a8630fb0921bc1203 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 11:47:43 -0700 Subject: [PATCH 03/23] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45558b0..587f1a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Release History +## 2.0.0b3 (Unreleased) + +* Fixes a bug where no allocation reason is set if a user is allocated to exactly 100. + ## 2.0.0b2 (10/11/2024) * Adds VariantAssignmentPercentage, DefaultWhenEnabled, and AllocationId to telemetry. From 628bb0bc6f2f1f2848bf7c2813c7017b6b5f7d61 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 11:50:11 -0700 Subject: [PATCH 04/23] update to b3 --- docs/conf.py | 2 +- featuremanagement/_version.py | 2 +- pyproject.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 0fd9889..c087c0f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -10,7 +10,7 @@ project = "FeatureManagement" copyright = "2024, Microsoft" author = "Microsoft" -release = "2.0.0b2" +release = "2.0.0b3" # -- General configuration --------------------------------------------------- diff --git a/featuremanagement/_version.py b/featuremanagement/_version.py index 99e8b1d..c6590f9 100644 --- a/featuremanagement/_version.py +++ b/featuremanagement/_version.py @@ -4,4 +4,4 @@ # license information. # ------------------------------------------------------------------------- -VERSION = "2.0.0b2" +VERSION = "2.0.0b3" diff --git a/pyproject.toml b/pyproject.toml index 8da891e..32b0689 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ build-backend = "setuptools.build_meta" [project] name = "FeatureManagement" -version = "2.0.0b2" +version = "2.0.0b3" authors = [ { name="Microsoft Corporation", email="appconfig@microsoft.com" }, ] From 9afcf33ed15a4d62481f8db19d5a75919f7e6240 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 11:59:46 -0700 Subject: [PATCH 05/23] cspell fixes --- project-words.txt | 1 + tests/test_send_telemetry_appinsights.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/project-words.txt b/project-words.txt index 7eca0be..20f4119 100644 --- a/project-words.txt +++ b/project-words.txt @@ -5,6 +5,7 @@ azuremonitor caplog Cass featurefilters +featureflag featuremanagement featuremanager featuremanagerbase diff --git a/tests/test_send_telemetry_appinsights.py b/tests/test_send_telemetry_appinsights.py index 636ee27..32110ed 100644 --- a/tests/test_send_telemetry_appinsights.py +++ b/tests/test_send_telemetry_appinsights.py @@ -21,8 +21,8 @@ def test_send_telemetry_appinsights(self): "metadata": { "ETag": "cmwBRcIAq1jUyKL3Kj8bvf9jtxBrFg-R-ayExStMC90", "FeatureFlagReference": "fake-store-uri/kv/.appconfig.featureflag/TestFeature", - "FeatureFlagId": "7vpkRJe452WVvlKXfA5XF3ASllwKsYZfC7D4w05rIoo", - "AllocationId": "MExY1waco2tqen4EcJKK", + "FeatureFlagId": "fake-feature-flag-id", + "AllocationId": "fake-allocation-id", }, }, } @@ -52,8 +52,8 @@ def test_send_telemetry_appinsights(self): mock_track_event.call_args[0][1]["FeatureFlagReference"] == "fake-store-uri/kv/.appconfig.featureflag/TestFeature" ) - assert mock_track_event.call_args[0][1]["FeatureFlagId"] == "7vpkRJe452WVvlKXfA5XF3ASllwKsYZfC7D4w05rIoo" - assert mock_track_event.call_args[0][1]["AllocationId"] == "MExY1waco2tqen4EcJKK" + assert mock_track_event.call_args[0][1]["FeatureFlagId"] == "fake-feature-flag-id" + assert mock_track_event.call_args[0][1]["AllocationId"] == "fake-allocation-id" assert "DefaultWhenEnabled" not in mock_track_event.call_args[0][1] def test_send_telemetry_appinsights_no_user(self): From 22005ccd8eaf0c00f962ce833fca7706bd7cd39b Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 13:30:35 -0700 Subject: [PATCH 06/23] Moving more shared code to base --- featuremanagement/_featuremanager.py | 35 +++------------- featuremanagement/_featuremanagerbase.py | 38 +++++++++++++++++ featuremanagement/aio/_featuremanager.py | 52 ++++++++---------------- tests/test_feature_manager.py | 9 +++- tests/test_feature_manager_async.py | 10 ++++- 5 files changed, 78 insertions(+), 66 deletions(-) diff --git a/featuremanagement/_featuremanager.py b/featuremanagement/_featuremanager.py index 80ba5ba..573fab8 100644 --- a/featuremanagement/_featuremanager.py +++ b/featuremanagement/_featuremanager.py @@ -3,16 +3,13 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # ------------------------------------------------------------------------- -import logging from typing import cast, overload, Any, Optional, Dict, Mapping, List from ._defaultfilters import TimeWindowFilter, TargetingFilter from ._featurefilters import FeatureFilter from ._models import EvaluationEvent, Variant, TargetingContext from ._featuremanagerbase import ( - _get_feature_flag, FeatureManagerBase, PROVIDED_FEATURE_FILTERS, - FEATURE_MANAGEMENT_KEY, REQUIREMENT_TYPE_ALL, FEATURE_FILTER_NAME, ) @@ -143,35 +140,13 @@ def _check_feature( Determine if the feature flag is enabled for the given context. :param str feature_flag_id: Name of the feature flag. - :return: True if the feature flag is enabled for the given context. - :rtype: bool + :param TargetingContext targeting_context: Targeting context. + :return: EvaluationEvent for the given context. + :rtype: EvaluationEvent """ - if self._copy is not self._configuration.get(FEATURE_MANAGEMENT_KEY): - self._cache = {} - self._copy = self._configuration.get(FEATURE_MANAGEMENT_KEY) - - if not self._cache.get(feature_flag_id): - feature_flag = _get_feature_flag(self._configuration, feature_flag_id) - self._cache[feature_flag_id] = feature_flag - else: - feature_flag = self._cache.get(feature_flag_id) - - evaluation_event = EvaluationEvent(feature_flag) - if not feature_flag: - logging.warning("Feature flag %s not found", feature_flag_id) - # Unknown feature flags are disabled by default - return evaluation_event - - if not feature_flag.enabled: - # Feature flags that are disabled are always disabled - FeatureManager._check_default_disabled_variant(evaluation_event) - if feature_flag.allocation: - variant_name = feature_flag.allocation.default_when_disabled - evaluation_event.variant = self._variant_name_to_variant(feature_flag, variant_name) - evaluation_event.feature = feature_flag + evaluation_event, done = FeatureManager._check_feature_base(self, feature_flag_id) - # If a feature flag is disabled and override can't enable it - evaluation_event.enabled = False + if done: return evaluation_event self._check_feature_filters(evaluation_event, targeting_context, **kwargs) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index 6816ec3..5a28159 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -4,6 +4,7 @@ # license information. # ------------------------------------------------------------------------- import hashlib +import logging from abc import ABC from typing import List, Optional, Dict, Tuple, Any, Mapping from ._models import FeatureFlag, Variant, VariantAssignmentReason, TargetingContext, EvaluationEvent, VariantReference @@ -261,6 +262,43 @@ def _assign_allocation(self, evaluation_event: EvaluationEvent, targeting_contex self._assign_variant(feature_flag, targeting_context, evaluation_event) + def _check_feature_base(self, feature_flag_id: str) -> Tuple[EvaluationEvent, bool]: + """ + Determine if the feature flag is enabled for the given context. + + :param str feature_flag_id: Name of the feature flag. + :return: The evaluation event and if the feature filters need to be checked. + :rtype: evaluation_event, bool + """ + if self._copy is not self._configuration.get(FEATURE_MANAGEMENT_KEY): + self._cache = {} + self._copy = self._configuration.get(FEATURE_MANAGEMENT_KEY) + + if not self._cache.get(feature_flag_id): + feature_flag = _get_feature_flag(self._configuration, feature_flag_id) + self._cache[feature_flag_id] = feature_flag + else: + feature_flag = self._cache.get(feature_flag_id) + + evaluation_event = EvaluationEvent(feature_flag) + if not feature_flag: + logging.warning("Feature flag %s not found", feature_flag_id) + # Unknown feature flags are disabled by default + return evaluation_event, True + + if not feature_flag.enabled: + # Feature flags that are disabled are always disabled + self._check_default_disabled_variant(evaluation_event) + if feature_flag.allocation: + variant_name = feature_flag.allocation.default_when_disabled + evaluation_event.variant = self._variant_name_to_variant(feature_flag, variant_name) + evaluation_event.feature = feature_flag + + # If a feature flag is disabled and override can't enable it + evaluation_event.enabled = False + return evaluation_event, True + return evaluation_event, False + def list_feature_flag_names(self) -> List[str]: """ List of all feature flag names. diff --git a/featuremanagement/aio/_featuremanager.py b/featuremanagement/aio/_featuremanager.py index 438e466..0f782d9 100644 --- a/featuremanagement/aio/_featuremanager.py +++ b/featuremanagement/aio/_featuremanager.py @@ -4,16 +4,13 @@ # license information. # ------------------------------------------------------------------------- import inspect -import logging -from typing import cast, overload, Mapping, Dict, Any, Optional, List +from typing import cast, overload, Any, Optional, Dict, Mapping, List from ._defaultfilters import TimeWindowFilter, TargetingFilter from ._featurefilters import FeatureFilter -from .._models import EvaluationEvent, TargetingContext, Variant +from .._models import EvaluationEvent, Variant, TargetingContext from .._featuremanagerbase import ( - _get_feature_flag, FeatureManagerBase, PROVIDED_FEATURE_FILTERS, - FEATURE_MANAGEMENT_KEY, REQUIREMENT_TYPE_ALL, FEATURE_FILTER_NAME, ) @@ -63,7 +60,12 @@ async def is_enabled(self, feature_flag_id: str, *args: Any, **kwargs: Any) -> b targeting_context = self._build_targeting_context(args) result = await self._check_feature(feature_flag_id, targeting_context, **kwargs) - if self._on_feature_evaluated and result.feature and result.feature.telemetry.enabled: + if ( + self._on_feature_evaluated + and result.feature + and result.feature.telemetry.enabled + and callable(self._on_feature_evaluated) + ): result.user = targeting_context.user_id if inspect.iscoroutinefunction(self._on_feature_evaluated): await self._on_feature_evaluated(result) @@ -94,7 +96,12 @@ async def get_variant(self, feature_flag_id: str, *args: Any, **kwargs: Any) -> targeting_context = self._build_targeting_context(args) result = await self._check_feature(feature_flag_id, targeting_context, **kwargs) - if self._on_feature_evaluated and result.feature and result.feature.telemetry.enabled: + if ( + self._on_feature_evaluated + and result.feature + and result.feature.telemetry.enabled + and callable(self._on_feature_evaluated) + ): result.user = targeting_context.user_id if inspect.iscoroutinefunction(self._on_feature_evaluated): await self._on_feature_evaluated(result) @@ -141,35 +148,12 @@ async def _check_feature( :param str feature_flag_id: Name of the feature flag. :param TargetingContext targeting_context: Targeting context. - :return: True if the feature flag is enabled for the given context. - :rtype: bool + :return: EvaluationEvent for the given context. + :rtype: EvaluationEvent """ - if self._copy is not self._configuration.get(FEATURE_MANAGEMENT_KEY): - self._cache = {} - self._copy = self._configuration.get(FEATURE_MANAGEMENT_KEY) - - if not self._cache.get(feature_flag_id): - feature_flag = _get_feature_flag(self._configuration, feature_flag_id) - self._cache[feature_flag_id] = feature_flag - else: - feature_flag = self._cache.get(feature_flag_id) - - evaluation_event = EvaluationEvent(feature_flag) - if not feature_flag: - logging.warning("Feature flag %s not found", feature_flag_id) - # Unknown feature flags are disabled by default - return evaluation_event - - if not feature_flag.enabled: - # Feature flags that are disabled are always disabled - FeatureManager._check_default_disabled_variant(evaluation_event) - if feature_flag.allocation: - variant_name = feature_flag.allocation.default_when_disabled - evaluation_event.variant = self._variant_name_to_variant(feature_flag, variant_name) - evaluation_event.feature = feature_flag + evaluation_event, done = FeatureManager._check_feature_base(self, feature_flag_id) - # If a feature flag is disabled and override can't enable it - evaluation_event.enabled = False + if done: return evaluation_event await self._check_feature_filters(evaluation_event, targeting_context, **kwargs) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index ecd52a3..4d7d541 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -4,10 +4,11 @@ # license information. # -------------------------------------------------------------------------- import pytest +import unittest from featuremanagement import FeatureManager, FeatureFilter -class TestFeatureManager: +class TestFeatureManager(unittest.TestCase): # method: feature_manager_creation def test_empty_feature_manager_creation(self): feature_manager = FeatureManager({}) @@ -29,6 +30,12 @@ def test_basic_feature_manager_creation(self): assert feature_manager.is_enabled("Alpha") assert not feature_manager.is_enabled("Beta") + # method: feature_manager_creation + def test_feature_manager_creation_invalid_feature_filter(self): + feature_flags = {"feature_management": {"feature_flags": []}} + with self.assertRaises(ValueError): + FeatureManager(feature_flags, feature_filters=["invalid_filter"]) + # method: feature_manager_creation def test_feature_manager_creation_with_filters(self): feature_flags = { diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 1e734c6..3e3dc6a 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -4,10 +4,11 @@ # license information. # -------------------------------------------------------------------------- import pytest +import unittest from featuremanagement.aio import FeatureManager, FeatureFilter -class TestFeatureManager: +class TestFeatureManager(unittest.IsolatedAsyncioTestCase): # method: feature_manager_creation @pytest.mark.asyncio async def test_empty_feature_manager_creation(self): @@ -32,6 +33,13 @@ async def test_basic_feature_manager_creation(self): assert await feature_manager.is_enabled("Alpha") assert not await feature_manager.is_enabled("Beta") + # method: feature_manager_creation + @pytest.mark.asyncio + def test_feature_manager_creation_invalid_feature_filter(self): + feature_flags = {"feature_management": {"feature_flags": []}} + with self.assertRaises(ValueError): + FeatureManager(feature_flags, feature_filters=["invalid_filter"]) + # method: feature_manager_creation @pytest.mark.asyncio async def test_feature_manager_creation_with_filters(self): From b6f492d5ba0564d5b3342486308fe94912279135 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 13:35:57 -0700 Subject: [PATCH 07/23] pylint fix --- tests/test_feature_manager.py | 2 +- tests/test_feature_manager_async.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index 4d7d541..d26e78a 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -3,8 +3,8 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # -------------------------------------------------------------------------- -import pytest import unittest +import pytest from featuremanagement import FeatureManager, FeatureFilter diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 3e3dc6a..51361d5 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -3,8 +3,8 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # -------------------------------------------------------------------------- -import pytest import unittest +import pytest from featuremanagement.aio import FeatureManager, FeatureFilter From 14d93bb0034f7a67a6176717f73d106a993fced2 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 14:01:50 -0700 Subject: [PATCH 08/23] more telemetry tests --- tests/test_feature_manager.py | 18 +++++++++ tests/test_feature_manager_async.py | 41 +++++++++++++++++++ tests/test_feature_variants.py | 30 ++++++++++++++ tests/test_feature_variants_async.py | 60 ++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index d26e78a..7ada123 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -126,6 +126,24 @@ def test_unknown_feature_filter(self): assert e_info.type == ValueError assert e_info.value.args[0] == "Feature flag Alpha has unknown filter UnknownFilter" + # method: feature_manager_creation + def test_feature_with_telemetry(self): + self.called_telemetry = False + feature_flags = { + "feature_management": { + "feature_flags": [ + {"id": "Alpha", "description": "", "enabled": "true", "telemetry": {"enabled": "true"}}, + ] + } + } + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + assert feature_manager is not None + assert feature_manager.is_enabled("Alpha") + assert self.called_telemetry + + def fake_telemetery_callback(self, evaluation_event): + self.called_telemetry = True + class AlwaysOn(FeatureFilter): def evaluate(self, context, **kwargs): diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 51361d5..feff3bb 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -134,6 +134,47 @@ async def test_unknown_feature_filter(self): assert e_info.type == ValueError assert e_info.value.args[0] == "Feature flag Alpha has unknown filter UnknownFilter" + # method: feature_manager_creation + @pytest.mark.asyncio + async def test_feature_with_telemetry(self): + self.called_telemetry = False + feature_flags = { + "feature_management": { + "feature_flags": [ + {"id": "Alpha", "description": "", "enabled": "true", "telemetry": {"enabled": "true"}}, + ] + } + } + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + assert feature_manager is not None + assert await feature_manager.is_enabled("Alpha") + assert self.called_telemetry + + def fake_telemetery_callback(self, evaluation_event): + self.called_telemetry = True + + # method: feature_manager_creation + @pytest.mark.asyncio + async def test_feature_with_telemetry_async(self): + self.called_telemetry = False + feature_flags = { + "feature_management": { + "feature_flags": [ + {"id": "Alpha", "description": "", "enabled": "true", "telemetry": {"enabled": "true"}}, + ] + } + } + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback_async) + assert feature_manager is not None + assert await feature_manager.is_enabled("Alpha") + assert self.called_telemetry + + def fake_telemetery_callback(self, evaluation_event): + self.called_telemetry = True + + async def fake_telemetery_callback_async(self, evaluation_event): + self.called_telemetry = True + class AlwaysOn(FeatureFilter): async def evaluate(self, context, **kwargs): diff --git a/tests/test_feature_variants.py b/tests/test_feature_variants.py index b1de322..84dd44a 100644 --- a/tests/test_feature_variants.py +++ b/tests/test_feature_variants.py @@ -285,6 +285,36 @@ def test_basic_feature_variant_allocation_percentile_seeded(self): assert not feature_manager.is_enabled("Alpha", "Dan") assert feature_manager.get_variant("Alpha", "Dan").name == "On" + # method: feature_manager_creation + def test_feature_with_telemetry(self): + self.called_telemetry = False + feature_flags = { + "feature_management": { + "feature_flags": [ + { + "id": "Alpha", + "enabled": True, + "variants": [ + {"name": "On", "status_override": "Disabled"}, + ], + "allocation": { + "default_when_enabled": "On", + }, + "telemetry": {"enabled": "true"}, + } + ] + } + } + + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + assert feature_manager is not None + assert not feature_manager.is_enabled("Alpha") + assert feature_manager.get_variant("Alpha").name == "On" + assert self.called_telemetry + + def fake_telemetery_callback(self, evaluation_event): + self.called_telemetry = True + class AlwaysOnFilter(FeatureFilter): def evaluate(self, context, **kwargs): diff --git a/tests/test_feature_variants_async.py b/tests/test_feature_variants_async.py index 3deafd8..d3a4141 100644 --- a/tests/test_feature_variants_async.py +++ b/tests/test_feature_variants_async.py @@ -251,6 +251,66 @@ async def test_basic_feature_variant_allocation_percentile_seeded(self): assert not await feature_manager.is_enabled("Alpha", "Dan") assert (await feature_manager.get_variant("Alpha", "Dan")).name == "On" + # method: feature_manager_creation + async def test_feature_with_telemetry(self): + self.called_telemetry = False + feature_flags = { + "feature_management": { + "feature_flags": [ + { + "id": "Alpha", + "enabled": True, + "variants": [ + {"name": "On", "status_override": "Disabled"}, + ], + "allocation": { + "default_when_enabled": "On", + }, + "telemetry": {"enabled": "true"}, + } + ] + } + } + + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + assert feature_manager is not None + assert not await feature_manager.is_enabled("Alpha") + assert (await feature_manager.get_variant("Alpha")).name == "On" + assert self.called_telemetry + + # method: feature_manager_creation + async def test_feature_with_telemetry_async(self): + self.called_telemetry = False + feature_flags = { + "feature_management": { + "feature_flags": [ + { + "id": "Alpha", + "enabled": True, + "variants": [ + {"name": "On", "status_override": "Disabled"}, + ], + "allocation": { + "default_when_enabled": "On", + }, + "telemetry": {"enabled": "true"}, + } + ] + } + } + + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback_async) + assert feature_manager is not None + assert not await feature_manager.is_enabled("Alpha") + assert (await feature_manager.get_variant("Alpha")).name == "On" + assert self.called_telemetry + + def fake_telemetery_callback(self, evaluation_event): + self.called_telemetry = True + + async def fake_telemetery_callback_async(self, evaluation_event): + self.called_telemetry = True + class AlwaysOnFilter(FeatureFilter): async def evaluate(self, context, **kwargs): From 6460aeb91772f73da280799195b6707b4e9ff219 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 14:04:32 -0700 Subject: [PATCH 09/23] spelling --- tests/test_feature_manager.py | 4 ++-- tests/test_feature_manager_async.py | 10 +++++----- tests/test_feature_variants.py | 4 ++-- tests/test_feature_variants_async.py | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index 7ada123..bffa39c 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -136,12 +136,12 @@ def test_feature_with_telemetry(self): ] } } - feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetry_callback) assert feature_manager is not None assert feature_manager.is_enabled("Alpha") assert self.called_telemetry - def fake_telemetery_callback(self, evaluation_event): + def fake_telemetry_callback(self, evaluation_event): self.called_telemetry = True diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index feff3bb..70bb4a7 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -145,12 +145,12 @@ async def test_feature_with_telemetry(self): ] } } - feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetry_callback) assert feature_manager is not None assert await feature_manager.is_enabled("Alpha") assert self.called_telemetry - def fake_telemetery_callback(self, evaluation_event): + def fake_telemetry_callback(self, evaluation_event): self.called_telemetry = True # method: feature_manager_creation @@ -164,15 +164,15 @@ async def test_feature_with_telemetry_async(self): ] } } - feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback_async) + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetry_callback_async) assert feature_manager is not None assert await feature_manager.is_enabled("Alpha") assert self.called_telemetry - def fake_telemetery_callback(self, evaluation_event): + def fake_telemetry_callback(self, evaluation_event): self.called_telemetry = True - async def fake_telemetery_callback_async(self, evaluation_event): + async def fake_telemetry_callback_async(self, evaluation_event): self.called_telemetry = True diff --git a/tests/test_feature_variants.py b/tests/test_feature_variants.py index 84dd44a..a7859de 100644 --- a/tests/test_feature_variants.py +++ b/tests/test_feature_variants.py @@ -306,13 +306,13 @@ def test_feature_with_telemetry(self): } } - feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetry_callback) assert feature_manager is not None assert not feature_manager.is_enabled("Alpha") assert feature_manager.get_variant("Alpha").name == "On" assert self.called_telemetry - def fake_telemetery_callback(self, evaluation_event): + def fake_telemetry_callback(self, evaluation_event): self.called_telemetry = True diff --git a/tests/test_feature_variants_async.py b/tests/test_feature_variants_async.py index d3a4141..7dc9d3f 100644 --- a/tests/test_feature_variants_async.py +++ b/tests/test_feature_variants_async.py @@ -272,7 +272,7 @@ async def test_feature_with_telemetry(self): } } - feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback) + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetry_callback) assert feature_manager is not None assert not await feature_manager.is_enabled("Alpha") assert (await feature_manager.get_variant("Alpha")).name == "On" @@ -299,13 +299,13 @@ async def test_feature_with_telemetry_async(self): } } - feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetery_callback_async) + feature_manager = FeatureManager(feature_flags, on_feature_evaluated=self.fake_telemetry_callback_async) assert feature_manager is not None assert not await feature_manager.is_enabled("Alpha") assert (await feature_manager.get_variant("Alpha")).name == "On" assert self.called_telemetry - def fake_telemetery_callback(self, evaluation_event): + def fake_telemetry_callback(self, evaluation_event): self.called_telemetry = True async def fake_telemetery_callback_async(self, evaluation_event): From 452aa49b5d5914f8dff73f1603fb8c8bcb4c7778 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 14:06:28 -0700 Subject: [PATCH 10/23] Update test_feature_variants_async.py --- tests/test_feature_variants_async.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_feature_variants_async.py b/tests/test_feature_variants_async.py index 7dc9d3f..0e6b5b0 100644 --- a/tests/test_feature_variants_async.py +++ b/tests/test_feature_variants_async.py @@ -308,7 +308,7 @@ async def test_feature_with_telemetry_async(self): def fake_telemetry_callback(self, evaluation_event): self.called_telemetry = True - async def fake_telemetery_callback_async(self, evaluation_event): + async def fake_telemetry_callback_async(self, evaluation_event): self.called_telemetry = True From 7ebfe76b883b726213ab2f0d873773621aa5ac69 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 14:12:36 -0700 Subject: [PATCH 11/23] fixing tests --- tests/test_feature_manager.py | 6 ++++++ tests/test_feature_manager_async.py | 10 +++++++--- tests/test_feature_variants.py | 6 ++++++ tests/test_feature_variants_async.py | 7 +++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index bffa39c..8351b6a 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -9,6 +9,11 @@ class TestFeatureManager(unittest.TestCase): + + def __init__(self): + super().__init__() + self.called_telemetry = False + # method: feature_manager_creation def test_empty_feature_manager_creation(self): feature_manager = FeatureManager({}) @@ -142,6 +147,7 @@ def test_feature_with_telemetry(self): assert self.called_telemetry def fake_telemetry_callback(self, evaluation_event): + assert evaluation_event self.called_telemetry = True diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 70bb4a7..10996ea 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -9,6 +9,11 @@ class TestFeatureManager(unittest.IsolatedAsyncioTestCase): + + def __init__(self): + super().__init__() + self.called_telemetry = False + # method: feature_manager_creation @pytest.mark.asyncio async def test_empty_feature_manager_creation(self): @@ -150,9 +155,6 @@ async def test_feature_with_telemetry(self): assert await feature_manager.is_enabled("Alpha") assert self.called_telemetry - def fake_telemetry_callback(self, evaluation_event): - self.called_telemetry = True - # method: feature_manager_creation @pytest.mark.asyncio async def test_feature_with_telemetry_async(self): @@ -170,9 +172,11 @@ async def test_feature_with_telemetry_async(self): assert self.called_telemetry def fake_telemetry_callback(self, evaluation_event): + assert evaluation_event self.called_telemetry = True async def fake_telemetry_callback_async(self, evaluation_event): + assert evaluation_event self.called_telemetry = True diff --git a/tests/test_feature_variants.py b/tests/test_feature_variants.py index a7859de..513afe8 100644 --- a/tests/test_feature_variants.py +++ b/tests/test_feature_variants.py @@ -7,6 +7,11 @@ class TestFeatureVariants: + + def __init__(self): + super().__init__() + self.called_telemetry = False + # method: is_enabled def test_basic_feature_variant_override_enabled(self): feature_flags = { @@ -313,6 +318,7 @@ def test_feature_with_telemetry(self): assert self.called_telemetry def fake_telemetry_callback(self, evaluation_event): + assert evaluation_event self.called_telemetry = True diff --git a/tests/test_feature_variants_async.py b/tests/test_feature_variants_async.py index 0e6b5b0..d1ffd42 100644 --- a/tests/test_feature_variants_async.py +++ b/tests/test_feature_variants_async.py @@ -9,6 +9,11 @@ class TestFeatureVariantsAsync(IsolatedAsyncioTestCase): + + def __init__(self): + super().__init__() + self.called_telemetry = False + # method: is_enabled async def test_basic_feature_variant_override_enabled(self): feature_flags = { @@ -306,9 +311,11 @@ async def test_feature_with_telemetry_async(self): assert self.called_telemetry def fake_telemetry_callback(self, evaluation_event): + assert evaluation_event self.called_telemetry = True async def fake_telemetry_callback_async(self, evaluation_event): + assert evaluation_event self.called_telemetry = True From 74845113f531e575d8ce4077d52a54fe1f27e330 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 14:19:26 -0700 Subject: [PATCH 12/23] fixing init --- tests/test_feature_manager.py | 4 ++-- tests/test_feature_manager_async.py | 4 ++-- tests/test_feature_variants.py | 7 ++++--- tests/test_feature_variants_async.py | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index 8351b6a..b2ac6e4 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -10,8 +10,8 @@ class TestFeatureManager(unittest.TestCase): - def __init__(self): - super().__init__() + def __init__(self, methodName='runTest'): + super().__init__(methodName=methodName) self.called_telemetry = False # method: feature_manager_creation diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 10996ea..c8625fa 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -10,8 +10,8 @@ class TestFeatureManager(unittest.IsolatedAsyncioTestCase): - def __init__(self): - super().__init__() + def __init__(self, methodName='runTest'): + super().__init__(methodName=methodName) self.called_telemetry = False # method: feature_manager_creation diff --git a/tests/test_feature_variants.py b/tests/test_feature_variants.py index 513afe8..5d6a626 100644 --- a/tests/test_feature_variants.py +++ b/tests/test_feature_variants.py @@ -3,13 +3,14 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # -------------------------------------------------------------------------- +import unittest from featuremanagement import FeatureManager, FeatureFilter, TargetingContext -class TestFeatureVariants: +class TestFeatureVariants(unittest.TestCase): - def __init__(self): - super().__init__() + def __init__(self, methodName='runTest'): + super().__init__(methodName=methodName) self.called_telemetry = False # method: is_enabled diff --git a/tests/test_feature_variants_async.py b/tests/test_feature_variants_async.py index d1ffd42..1a7cf5e 100644 --- a/tests/test_feature_variants_async.py +++ b/tests/test_feature_variants_async.py @@ -10,8 +10,8 @@ class TestFeatureVariantsAsync(IsolatedAsyncioTestCase): - def __init__(self): - super().__init__() + def __init__(self, methodName='runTest'): + super().__init__(methodName=methodName) self.called_telemetry = False # method: is_enabled From 8c2bd9c8db770830954fe11e75c96d9549d25442 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 17 Oct 2024 14:29:08 -0700 Subject: [PATCH 13/23] inits --- tests/test_feature_manager.py | 2 +- tests/test_feature_manager_async.py | 2 +- tests/test_feature_variants.py | 2 +- tests/test_feature_variants_async.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index b2ac6e4..7813835 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -10,7 +10,7 @@ class TestFeatureManager(unittest.TestCase): - def __init__(self, methodName='runTest'): + def __init__(self, methodName="runTest"): super().__init__(methodName=methodName) self.called_telemetry = False diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index c8625fa..1f6c959 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -10,7 +10,7 @@ class TestFeatureManager(unittest.IsolatedAsyncioTestCase): - def __init__(self, methodName='runTest'): + def __init__(self, methodName="runTest"): super().__init__(methodName=methodName) self.called_telemetry = False diff --git a/tests/test_feature_variants.py b/tests/test_feature_variants.py index 5d6a626..860d70b 100644 --- a/tests/test_feature_variants.py +++ b/tests/test_feature_variants.py @@ -9,7 +9,7 @@ class TestFeatureVariants(unittest.TestCase): - def __init__(self, methodName='runTest'): + def __init__(self, methodName="runTest"): super().__init__(methodName=methodName) self.called_telemetry = False diff --git a/tests/test_feature_variants_async.py b/tests/test_feature_variants_async.py index 1a7cf5e..9b4a71c 100644 --- a/tests/test_feature_variants_async.py +++ b/tests/test_feature_variants_async.py @@ -10,7 +10,7 @@ class TestFeatureVariantsAsync(IsolatedAsyncioTestCase): - def __init__(self, methodName='runTest'): + def __init__(self, methodName="runTest"): super().__init__(methodName=methodName) self.called_telemetry = False From 53750d92a6e40a55d5ecce8f98024f97b707bfa0 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 21 Oct 2024 09:21:55 -0700 Subject: [PATCH 14/23] Update _send_telemetry.py --- featuremanagement/azuremonitor/_send_telemetry.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index f0941fb..902c2fa 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -83,13 +83,12 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: and feature.allocation.percentile ): for allocation in feature.allocation.percentile: - if variant and allocation.variant != variant.name and allocation.percentile_to: + if variant and allocation.variant == variant.name and allocation.percentile_to: allocation_percentage += allocation.percentile_to - allocation.percentile_from event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) elif reason == VariantAssignmentReason.PERCENTILE: if feature.allocation and feature.allocation.percentile: - for allocation in feature.allocation.percentile: if variant and allocation.variant == variant.name and allocation.percentile_to: allocation_percentage += allocation.percentile_to - allocation.percentile_from From d86d120f4f36e7d395d745a4f0bb11474fda04b2 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 21 Oct 2024 11:28:26 -0700 Subject: [PATCH 15/23] fixes bug where default allocation calc is wrong --- featuremanagement/azuremonitor/_send_telemetry.py | 2 +- tests/test_send_telemetry_appinsights.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index 902c2fa..128ef77 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -83,7 +83,7 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: and feature.allocation.percentile ): for allocation in feature.allocation.percentile: - if variant and allocation.variant == variant.name and allocation.percentile_to: + if allocation.percentile_to: allocation_percentage += allocation.percentile_to - allocation.percentile_from event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) diff --git a/tests/test_send_telemetry_appinsights.py b/tests/test_send_telemetry_appinsights.py index 32110ed..219ebf7 100644 --- a/tests/test_send_telemetry_appinsights.py +++ b/tests/test_send_telemetry_appinsights.py @@ -141,7 +141,7 @@ def test_send_telemetry_appinsights_default_when_enabled(self): assert mock_track_event.call_args[0][1]["TargetingId"] == "test_user" assert mock_track_event.call_args[0][1]["Variant"] == "big" assert mock_track_event.call_args[0][1]["VariantAssignmentReason"] == "DefaultWhenEnabled" - assert mock_track_event.call_args[0][1]["VariantAssignmentPercentage"] == "50" + assert mock_track_event.call_args[0][1]["VariantAssignmentPercentage"] == "25" assert "DefaultWhenEnabled" in mock_track_event.call_args[0][1] assert mock_track_event.call_args[0][1]["DefaultWhenEnabled"] == "big" From b95b548deb4ab56ba45f1881f726beba917ae694 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 21 Oct 2024 11:39:31 -0700 Subject: [PATCH 16/23] Update _send_telemetry.py --- .../azuremonitor/_send_telemetry.py | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index 128ef77..26a4755 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -71,28 +71,25 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: variant = evaluation_event.variant # VariantAllocationPercentage - if reason and reason != VariantAssignmentReason.NONE: - if variant: - event[VARIANT] = variant.name + if reason: event[REASON] = reason.value - allocation_percentage = 0 - if ( - reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED - and feature.allocation - and feature.allocation.percentile - ): + if variant: + event[VARIANT] = variant.name + + allocation_percentage = 0 + if reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED and feature.allocation and feature.allocation.percentile: + for allocation in feature.allocation.percentile: + if allocation.percentile_to: + allocation_percentage += allocation.percentile_to - allocation.percentile_from + + event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) + elif reason == VariantAssignmentReason.PERCENTILE: + if feature.allocation and feature.allocation.percentile: for allocation in feature.allocation.percentile: - if allocation.percentile_to: + if variant and allocation.variant == variant.name and allocation.percentile_to: allocation_percentage += allocation.percentile_to - allocation.percentile_from - - event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) - elif reason == VariantAssignmentReason.PERCENTILE: - if feature.allocation and feature.allocation.percentile: - for allocation in feature.allocation.percentile: - if variant and allocation.variant == variant.name and allocation.percentile_to: - allocation_percentage += allocation.percentile_to - allocation.percentile_from - event["VariantAssignmentPercentage"] = str(allocation_percentage) + event["VariantAssignmentPercentage"] = str(allocation_percentage) # DefaultWhenEnabled if feature.allocation and feature.allocation.default_when_enabled: From 0655a7211c9c0855ff52494698dd19909c0016ed Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 21 Oct 2024 11:45:31 -0700 Subject: [PATCH 17/23] modifying the default value of percentile_to --- featuremanagement/_featuremanagerbase.py | 7 +++---- featuremanagement/_models/_allocation.py | 4 ++-- featuremanagement/azuremonitor/_send_telemetry.py | 7 +++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index 5a28159..ff25a73 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -185,10 +185,9 @@ def _assign_variant( context_id = f"{targeting_context.user_id}\n{seed}" box: float = self._is_targeted(context_id) for percentile_allocation in allocation.percentile: - percentile_to: Optional[int] = percentile_allocation.percentile_to - if percentile_to is not None and ( - (box == 100 and percentile_to == 100) - or (percentile_allocation.percentile_from <= box < percentile_to) + percentile_to: int = percentile_allocation.percentile_to + if (box == 100 and percentile_to == 100) or ( + percentile_allocation.percentile_from <= box < percentile_to ): evaluation_event.reason = VariantAssignmentReason.PERCENTILE variant_name = percentile_allocation.variant diff --git a/featuremanagement/_models/_allocation.py b/featuremanagement/_models/_allocation.py index 2ce0fb6..3930a6c 100644 --- a/featuremanagement/_models/_allocation.py +++ b/featuremanagement/_models/_allocation.py @@ -36,7 +36,7 @@ class PercentileAllocation: def __init__(self) -> None: self._variant: Optional[str] = None self._percentile_from: int = 0 - self._percentile_to: Optional[int] = None + self._percentile_to: int = 100 @classmethod def convert_from_json(cls, json: Mapping[str, Union[str, int]]) -> "PercentileAllocation": @@ -88,7 +88,7 @@ def percentile_from(self) -> int: return self._percentile_from @property - def percentile_to(self) -> Optional[int]: + def percentile_to(self) -> int: """ Get the ending percentile for the allocation. diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index 26a4755..174e984 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -70,24 +70,23 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: reason = evaluation_event.reason variant = evaluation_event.variant - # VariantAllocationPercentage if reason: event[REASON] = reason.value if variant: event[VARIANT] = variant.name + # VariantAllocationPercentage allocation_percentage = 0 if reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED and feature.allocation and feature.allocation.percentile: for allocation in feature.allocation.percentile: - if allocation.percentile_to: - allocation_percentage += allocation.percentile_to - allocation.percentile_from + allocation_percentage += allocation.percentile_to - allocation.percentile_from event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) elif reason == VariantAssignmentReason.PERCENTILE: if feature.allocation and feature.allocation.percentile: for allocation in feature.allocation.percentile: - if variant and allocation.variant == variant.name and allocation.percentile_to: + if variant and allocation.variant == variant.name: allocation_percentage += allocation.percentile_to - allocation.percentile_from event["VariantAssignmentPercentage"] = str(allocation_percentage) From a8486e37d590371d2110e19fb2c3d7a80835434f Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 22 Oct 2024 15:20:58 -0700 Subject: [PATCH 18/23] Update _allocation.py --- featuremanagement/_models/_allocation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/featuremanagement/_models/_allocation.py b/featuremanagement/_models/_allocation.py index 3930a6c..09dba6a 100644 --- a/featuremanagement/_models/_allocation.py +++ b/featuremanagement/_models/_allocation.py @@ -36,7 +36,7 @@ class PercentileAllocation: def __init__(self) -> None: self._variant: Optional[str] = None self._percentile_from: int = 0 - self._percentile_to: int = 100 + self._percentile_to: int = 0 @classmethod def convert_from_json(cls, json: Mapping[str, Union[str, int]]) -> "PercentileAllocation": From e0fe682d20176d4b28aab55aebccf3edb28ca61d Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Wed, 23 Oct 2024 09:24:42 -0700 Subject: [PATCH 19/23] changed to super --- featuremanagement/_featuremanager.py | 2 +- featuremanagement/aio/_featuremanager.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/featuremanagement/_featuremanager.py b/featuremanagement/_featuremanager.py index 573fab8..0a48da8 100644 --- a/featuremanagement/_featuremanager.py +++ b/featuremanagement/_featuremanager.py @@ -144,7 +144,7 @@ def _check_feature( :return: EvaluationEvent for the given context. :rtype: EvaluationEvent """ - evaluation_event, done = FeatureManager._check_feature_base(self, feature_flag_id) + evaluation_event, done = super()._check_feature_base(feature_flag_id) if done: return evaluation_event diff --git a/featuremanagement/aio/_featuremanager.py b/featuremanagement/aio/_featuremanager.py index 0f782d9..fd7db5e 100644 --- a/featuremanagement/aio/_featuremanager.py +++ b/featuremanagement/aio/_featuremanager.py @@ -151,7 +151,7 @@ async def _check_feature( :return: EvaluationEvent for the given context. :rtype: EvaluationEvent """ - evaluation_event, done = FeatureManager._check_feature_base(self, feature_flag_id) + evaluation_event, done = super()._check_feature_base(feature_flag_id) if done: return evaluation_event From a4a66f667aebccb7128677c87b0146785535e0c6 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Wed, 23 Oct 2024 09:34:08 -0700 Subject: [PATCH 20/23] removed extra feature flag instance --- featuremanagement/_featuremanagerbase.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index ff25a73..fbc7e11 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -150,22 +150,19 @@ def _is_targeted(context_id: str) -> float: return (context_marker / (2**32 - 1)) * 100 - def _assign_variant( - self, feature_flag: FeatureFlag, targeting_context: TargetingContext, evaluation_event: EvaluationEvent - ) -> None: + def _assign_variant(self, targeting_context: TargetingContext, evaluation_event: EvaluationEvent) -> None: """ Assign a variant to the user based on the allocation. - :param FeatureFlag feature_flag: Feature flag object. :param TargetingContext targeting_context: Targeting context. :param EvaluationEvent evaluation_event: Evaluation event object. """ - feature = evaluation_event.feature + feature_flag = evaluation_event.feature variant_name = None - if not feature or not feature.variants or not feature.allocation: + if not feature_flag or not feature_flag.variants or not feature_flag.allocation: return - allocation = feature.allocation + allocation = feature_flag.allocation groups = targeting_context.groups if allocation.user and targeting_context.user_id: @@ -181,7 +178,7 @@ def _assign_variant( variant_name = group_allocation.variant if not variant_name and allocation.percentile: - seed = allocation.seed or f"allocation\n{feature.name}" + seed = allocation.seed or f"allocation\n{feature_flag.name}" context_id = f"{targeting_context.user_id}\n{seed}" box: float = self._is_targeted(context_id) for percentile_allocation in allocation.percentile: @@ -259,7 +256,7 @@ def _assign_allocation(self, evaluation_event: EvaluationEvent, targeting_contex ) return - self._assign_variant(feature_flag, targeting_context, evaluation_event) + self._assign_variant(targeting_context, evaluation_event) def _check_feature_base(self, feature_flag_id: str) -> Tuple[EvaluationEvent, bool]: """ From 2045f125fc073286f40c48107b33bfac6da7c34f Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Wed, 23 Oct 2024 12:05:40 -0700 Subject: [PATCH 21/23] review comments --- featuremanagement/_featuremanagerbase.py | 19 +++++++++---------- featuremanagement/aio/_featuremanager.py | 4 ++-- .../azuremonitor/_send_telemetry.py | 1 - 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index fbc7e11..350174f 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -79,7 +79,7 @@ def __init__(self, configuration: Mapping[str, Any], **kwargs: Any): self._on_feature_evaluated = kwargs.pop("on_feature_evaluated", None) @staticmethod - def _check_default_disabled_variant(evaluation_event: EvaluationEvent) -> None: + def _assign_default_disabled_variant(evaluation_event: EvaluationEvent) -> None: """ A method called when the feature flag is disabled, to determine what the default variant should be. If there is no allocation, then None is set as the value of the variant in the EvaluationEvent. @@ -89,7 +89,7 @@ def _check_default_disabled_variant(evaluation_event: EvaluationEvent) -> None: evaluation_event.reason = VariantAssignmentReason.DEFAULT_WHEN_DISABLED if not evaluation_event.feature or not evaluation_event.feature.allocation: return - FeatureManagerBase._check_variant_override( + FeatureManagerBase._assign_variant_override( evaluation_event.feature.variants, evaluation_event.feature.allocation.default_when_disabled, False, @@ -97,7 +97,7 @@ def _check_default_disabled_variant(evaluation_event: EvaluationEvent) -> None: ) @staticmethod - def _check_default_enabled_variant(evaluation_event: EvaluationEvent) -> None: + def _assign_default_enabled_variant(evaluation_event: EvaluationEvent) -> None: """ A method called when the feature flag is enabled, to determine what the default variant should be. If there is no allocation, then None is set as the value of the variant in the EvaluationEvent. @@ -107,7 +107,7 @@ def _check_default_enabled_variant(evaluation_event: EvaluationEvent) -> None: evaluation_event.reason = VariantAssignmentReason.DEFAULT_WHEN_ENABLED if not evaluation_event.feature or not evaluation_event.feature.allocation: return - FeatureManagerBase._check_variant_override( + FeatureManagerBase._assign_variant_override( evaluation_event.feature.variants, evaluation_event.feature.allocation.default_when_enabled, True, @@ -115,7 +115,7 @@ def _check_default_enabled_variant(evaluation_event: EvaluationEvent) -> None: ) @staticmethod - def _check_variant_override( + def _assign_variant_override( variants: Optional[List[VariantReference]], default_variant_name: Optional[str], status: bool, @@ -191,7 +191,7 @@ def _assign_variant(self, targeting_context: TargetingContext, evaluation_event: break if not variant_name: - FeatureManagerBase._check_default_enabled_variant(evaluation_event) + FeatureManagerBase._assign_default_enabled_variant(evaluation_event) if feature_flag.allocation: evaluation_event.variant = self._variant_name_to_variant( feature_flag, feature_flag.allocation.default_when_enabled @@ -200,7 +200,7 @@ def _assign_variant(self, targeting_context: TargetingContext, evaluation_event: evaluation_event.variant = self._variant_name_to_variant(feature_flag, variant_name) if feature_flag.variants: - FeatureManagerBase._check_variant_override(feature_flag.variants, variant_name, True, evaluation_event) + FeatureManagerBase._assign_variant_override(feature_flag.variants, variant_name, True, evaluation_event) def _variant_name_to_variant(self, feature_flag: FeatureFlag, variant_name: Optional[str]) -> Optional[Variant]: """ @@ -250,7 +250,7 @@ def _assign_allocation(self, evaluation_event: EvaluationEvent, targeting_contex ) return if not evaluation_event.enabled: - FeatureManagerBase._check_default_disabled_variant(evaluation_event) + FeatureManagerBase._assign_default_disabled_variant(evaluation_event) evaluation_event.variant = self._variant_name_to_variant( feature_flag, feature_flag.allocation.default_when_disabled ) @@ -284,11 +284,10 @@ def _check_feature_base(self, feature_flag_id: str) -> Tuple[EvaluationEvent, bo if not feature_flag.enabled: # Feature flags that are disabled are always disabled - self._check_default_disabled_variant(evaluation_event) + self._assign_default_disabled_variant(evaluation_event) if feature_flag.allocation: variant_name = feature_flag.allocation.default_when_disabled evaluation_event.variant = self._variant_name_to_variant(feature_flag, variant_name) - evaluation_event.feature = feature_flag # If a feature flag is disabled and override can't enable it evaluation_event.enabled = False diff --git a/featuremanagement/aio/_featuremanager.py b/featuremanagement/aio/_featuremanager.py index fd7db5e..d2cce03 100644 --- a/featuremanagement/aio/_featuremanager.py +++ b/featuremanagement/aio/_featuremanager.py @@ -69,7 +69,7 @@ async def is_enabled(self, feature_flag_id: str, *args: Any, **kwargs: Any) -> b result.user = targeting_context.user_id if inspect.iscoroutinefunction(self._on_feature_evaluated): await self._on_feature_evaluated(result) - elif callable(self._on_feature_evaluated): + else: self._on_feature_evaluated(result) return result.enabled @@ -105,7 +105,7 @@ async def get_variant(self, feature_flag_id: str, *args: Any, **kwargs: Any) -> result.user = targeting_context.user_id if inspect.iscoroutinefunction(self._on_feature_evaluated): await self._on_feature_evaluated(result) - elif callable(self._on_feature_evaluated): + else: self._on_feature_evaluated(result) return result.variant diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index 174e984..be8f12d 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -81,7 +81,6 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: if reason == VariantAssignmentReason.DEFAULT_WHEN_ENABLED and feature.allocation and feature.allocation.percentile: for allocation in feature.allocation.percentile: allocation_percentage += allocation.percentile_to - allocation.percentile_from - event["VariantAssignmentPercentage"] = str(100 - allocation_percentage) elif reason == VariantAssignmentReason.PERCENTILE: if feature.allocation and feature.allocation.percentile: From 4cab8aa6b8212f83aa5ab345000010aafd5bb747 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 24 Oct 2024 09:56:05 -0700 Subject: [PATCH 22/23] Evaluation Event defeult to NONE --- featuremanagement/_models/_evaluation_event.py | 2 +- featuremanagement/azuremonitor/_send_telemetry.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/featuremanagement/_models/_evaluation_event.py b/featuremanagement/_models/_evaluation_event.py index af18a63..c0d7827 100644 --- a/featuremanagement/_models/_evaluation_event.py +++ b/featuremanagement/_models/_evaluation_event.py @@ -24,4 +24,4 @@ def __init__(self, feature_flag: Optional[FeatureFlag]): self.user = "" self.enabled = False self.variant: Optional[Variant] = None - self.reason: Optional[VariantAssignmentReason] = None + self.reason: VariantAssignmentReason = VariantAssignmentReason.NONE diff --git a/featuremanagement/azuremonitor/_send_telemetry.py b/featuremanagement/azuremonitor/_send_telemetry.py index be8f12d..8655fd8 100644 --- a/featuremanagement/azuremonitor/_send_telemetry.py +++ b/featuremanagement/azuremonitor/_send_telemetry.py @@ -70,8 +70,7 @@ def publish_telemetry(evaluation_event: EvaluationEvent) -> None: reason = evaluation_event.reason variant = evaluation_event.variant - if reason: - event[REASON] = reason.value + event[REASON] = reason.value if variant: event[VARIANT] = variant.name From 55fba0a750f077b8ddfa3ae66b988edacba10d7d Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 24 Oct 2024 15:06:44 -0700 Subject: [PATCH 23/23] removing unneeded code --- featuremanagement/_featuremanagerbase.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index 350174f..5078caf 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -239,16 +239,9 @@ def _assign_allocation(self, evaluation_event: EvaluationEvent, targeting_contex if not feature_flag: return - if not feature_flag.variants: + if not feature_flag.variants or not feature_flag.allocation: return - if not feature_flag.allocation: - evaluation_event.reason = ( - VariantAssignmentReason.DEFAULT_WHEN_ENABLED - if evaluation_event.enabled - else VariantAssignmentReason.DEFAULT_WHEN_DISABLED - ) - return if not evaluation_event.enabled: FeatureManagerBase._assign_default_disabled_variant(evaluation_event) evaluation_event.variant = self._variant_name_to_variant(