Skip to content

feat: strategy variants #265

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
1ab01cf
feat: strategy variants
andreas-unleash Jul 28, 2023
f808c9e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2023
753d84f
fix linter
andreas-unleash Jul 28, 2023
0472a73
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Jul 28, 2023
beb5e3c
add unit tests
andreas-unleash Jul 31, 2023
924cc8c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 31, 2023
6099ada
add unit tests
andreas-unleash Jul 31, 2023
5463aae
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Jul 31, 2023
1067487
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 31, 2023
fdb7ea5
lint
andreas-unleash Jul 31, 2023
4ce8910
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Jul 31, 2023
9802a65
cleanup
andreas-unleash Jul 31, 2023
61e3c1f
cleanup
andreas-unleash Jul 31, 2023
04598ca
cleanup
andreas-unleash Jul 31, 2023
92ebb9d
cleanup
andreas-unleash Jul 31, 2023
08c0d6e
fix linter complaints
andreas-unleash Jul 31, 2023
79fdba2
fix linter complaints
andreas-unleash Jul 31, 2023
6176ad2
fix linter complaints
andreas-unleash Jul 31, 2023
737a1a9
fix logic
andreas-unleash Jul 31, 2023
3a3ff93
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 31, 2023
8deaffc
formatting
andreas-unleash Jul 31, 2023
3e85407
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Jul 31, 2023
68a1373
improvements
andreas-unleash Aug 1, 2023
aed11a4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 1, 2023
c435de8
bug fixes
andreas-unleash Aug 2, 2023
4092c21
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Aug 2, 2023
ca4e918
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 2, 2023
cae4aff
remove constraint noise from test
andreas-unleash Aug 2, 2023
1d1f438
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Aug 2, 2023
a3e2c77
fix tests
andreas-unleash Aug 2, 2023
59f5568
fix tests
andreas-unleash Aug 2, 2023
25e10dc
fix ruff complaining
andreas-unleash Aug 2, 2023
00f2c44
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 2, 2023
09e205c
fix PR comments
andreas-unleash Aug 3, 2023
2dcfec8
Merge remote-tracking branch 'origin/feat/strategy_variants' into fea…
andreas-unleash Aug 3, 2023
ecbf360
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion UnleashClient/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ def get_variant(self, feature_name: str, context: Optional[dict] = None) -> dict
self.features[feature_name] = new_feature

# Use the feature's get_variant method to count the call
return new_feature.get_variant(context)
variant_check = new_feature.get_variant(context)
return variant_check
else:
LOGGER.log(
self.unleash_verbose_log_level,
Expand Down
2 changes: 1 addition & 1 deletion UnleashClient/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
REQUEST_TIMEOUT = 30
REQUEST_RETRIES = 3
METRIC_LAST_SENT_TIME = "mlst"
CLIENT_SPEC_VERSION = "4.2.2"
CLIENT_SPEC_VERSION = "4.3.1"

# =Unleash=
APPLICATION_HEADERS = {
Expand Down
7 changes: 3 additions & 4 deletions UnleashClient/constraints/Constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,9 @@ def apply(self, context: dict = None) -> bool:
constraint_check = self.check_semver_operators(
context_value=context_value
)
else:
# This is a special case in the client spec - so it's getting it's own handler here
if self.operator is ConstraintOperators.NOT_IN: # noqa: PLR5501
constraint_check = True
# This is a special case in the client spec - so it's getting it's own handler here
elif self.operator is ConstraintOperators.NOT_IN: # noqa: PLR5501
constraint_check = True

except Exception as excep: # pylint: disable=broad-except
LOGGER.info(
Expand Down
2 changes: 1 addition & 1 deletion UnleashClient/deprecation_warnings.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def strategy_v2xx_deprecation_check(strategies: list) -> None:
for strategy in strategies:
try:
# Check if the __call__() method is overwritten (should only be true for custom strategies in v1.x or v2.x.
if strategy.__call__ != Strategy.__call__:
if strategy.__call__ != Strategy.__call__: # type:ignore
warnings.warn(
f"unleash-client-python v3.x.x requires overriding the execute() method instead of the __call__() method. Error in: {strategy.__name__}",
DeprecationWarning,
Expand Down
64 changes: 38 additions & 26 deletions UnleashClient/features/Feature.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# pylint: disable=invalid-name
import copy
from typing import Dict, Optional, cast

from UnleashClient.constants import DISABLED_VARIATION
from UnleashClient.strategies import EvaluationResult, Strategy
from UnleashClient.utils import LOGGER
from UnleashClient.variants import Variants

Expand Down Expand Up @@ -73,33 +75,16 @@ def _count_variant(self, variant_name: str) -> None:
"""
self.variant_counts[variant_name] = self.variant_counts.get(variant_name, 0) + 1

def is_enabled(
self, context: dict = None, default_value: bool = False
) -> bool: # pylint: disable=unused-argument
def is_enabled(self, context: dict = None) -> bool:
"""
Checks if feature is enabled.

:param context: Context information
:param default_value: Deprecated! Users should use the fallback_function on the main is_enabled() method.
:return:
"""
flag_value = False
evaluation_result = self._get_evaluation_result(context)

if self.enabled:
try:
if self.strategies:
strategy_result = any(x.execute(context) for x in self.strategies)
else:
# If no strategies are present, should default to true. This isn't possible via UI.
strategy_result = True

flag_value = strategy_result
except Exception as strategy_except:
LOGGER.warning("Error checking feature flag: %s", strategy_except)

self.increment_stats(flag_value)

LOGGER.info("Feature toggle status for feature %s: %s", self.name, flag_value)
flag_value = evaluation_result.enabled

return flag_value

Expand All @@ -110,19 +95,46 @@ def get_variant(self, context: dict = None) -> dict:
:param context: Context information
:return:
"""
variant = DISABLED_VARIATION
is_feature_enabled = self.is_enabled(context)

if is_feature_enabled and self.variants is not None:
evaluation_result = self._get_evaluation_result(context)
is_feature_enabled = evaluation_result.enabled
variant = evaluation_result.variant
if variant is None or (is_feature_enabled and variant == DISABLED_VARIATION):
try:
variant = self.variants.get_variant(context)
variant["enabled"] = is_feature_enabled
LOGGER.debug("Getting variant from feature: %s", self.name)
variant = (
self.variants.get_variant(context, is_feature_enabled)
if is_feature_enabled
else copy.deepcopy(DISABLED_VARIATION)
)

except Exception as variant_exception:
LOGGER.warning("Error selecting variant: %s", variant_exception)

self._count_variant(cast(str, variant["name"]))
return variant

def _get_evaluation_result(self, context: dict = None) -> EvaluationResult:
strategy_result = EvaluationResult(False, None)
if self.enabled:
try:
if self.strategies:
enabled_strategy: Strategy = next(
(x for x in self.strategies if x.execute(context)), None
)
if enabled_strategy is not None:
strategy_result = enabled_strategy.get_result(context)

else:
# If no strategies are present, should default to true. This isn't possible via UI.
strategy_result = EvaluationResult(True, None)

except Exception as evaluation_except:
LOGGER.warning("Error getting evaluation result: %s", evaluation_except)

self.increment_stats(strategy_result.enabled)
LOGGER.info("%s evaluation result: %s", self.name, strategy_result)
return strategy_result

@staticmethod
def metrics_only_feature(feature_name: str):
feature = Feature(feature_name, False, [])
Expand Down
6 changes: 6 additions & 0 deletions UnleashClient/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,15 @@ def _create_strategies(
else:
segment_provisioning = []

if "variants" in strategy.keys():
variant_provisioning = strategy["variants"]
else:
variant_provisioning = []

feature_strategies.append(
strategy_mapping[strategy["name"]](
constraints=constraint_provisioning,
variants=variant_provisioning,
parameters=strategy_provisioning,
global_segments=global_segments,
segment_ids=segment_provisioning,
Expand Down
7 changes: 3 additions & 4 deletions UnleashClient/strategies/RemoteAddress.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ def apply(self, context: dict = None) -> bool:
if context_ip == addr_or_range:
return_value = True
break
else:
if context_ip in addr_or_range: # noqa: PLR5501
return_value = True
break
elif context_ip in addr_or_range: # noqa: PLR5501
return_value = True
break

return return_value
30 changes: 29 additions & 1 deletion UnleashClient/strategies/Strategy.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
# pylint: disable=invalid-name,dangerous-default-value
import warnings
from typing import Iterator
from dataclasses import dataclass
from typing import Iterator, Optional

from UnleashClient.constraints import Constraint
from UnleashClient.variants import Variants


@dataclass
class EvaluationResult:
enabled: bool
variant: Optional[dict]


class Strategy:
Expand All @@ -15,6 +23,7 @@ class Strategy:
- ``apply()`` - Your feature provisioning

:param constraints: List of 'constraints' objects derived from strategy section (...from feature section) of `/api/clients/features` Unleash server response.
:param variants: List of 'variant' objects derived from strategy section (...from feature section) of `/api/clients/features` Unleash server response.
:param parameters: The 'parameter' objects from the strategy section (...from feature section) of `/api/clients/features` Unleash server response.
"""

Expand All @@ -24,9 +33,11 @@ def __init__(
parameters: dict = {},
segment_ids: list = None,
global_segments: dict = None,
variants: list = None,
) -> None:
self.parameters = parameters
self.constraints = constraints
self.variants = variants or []
self.segment_ids = segment_ids or []
self.global_segments = global_segments or {}
self.parsed_provisioning = self.load_provisioning()
Expand Down Expand Up @@ -56,6 +67,15 @@ def execute(self, context: dict = None) -> bool:

return flag_state

def get_result(self, context) -> EvaluationResult:
enabled = self.execute(context)
variant = None
if enabled:
variant = self.parsed_variants.get_variant(context, enabled)

result = EvaluationResult(enabled, variant)
return result

@property
def parsed_constraints(self) -> Iterator[Constraint]:
for constraint_dict in self.constraints:
Expand All @@ -66,6 +86,14 @@ def parsed_constraints(self) -> Iterator[Constraint]:
for constraint in segment["constraints"]:
yield Constraint(constraint_dict=constraint)

@property
def parsed_variants(self) -> Variants:
return Variants(
variants_list=self.variants,
group_id=self.parameters.get("groupId"),
is_feature_variants=False,
)

def load_provisioning(self) -> list: # pylint: disable=no-self-use
"""
Loads strategy provisioning from Unleash feature flag configuration.
Expand Down
2 changes: 1 addition & 1 deletion UnleashClient/strategies/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
from .GradualRolloutSessionId import GradualRolloutSessionId
from .GradualRolloutUserId import GradualRolloutUserId
from .RemoteAddress import RemoteAddress
from .Strategy import Strategy
from .Strategy import EvaluationResult, Strategy
from .UserWithId import UserWithId
23 changes: 15 additions & 8 deletions UnleashClient/variants/Variants.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
# pylint: disable=invalid-name, too-few-public-methods
import copy
import random
from typing import Dict # noqa: F401
from typing import Dict, Optional # noqa: F401

from UnleashClient import utils
from UnleashClient.constants import DISABLED_VARIATION


class Variants:
def __init__(self, variants_list: list, feature_name: str) -> None:
def __init__(
self, variants_list: list, group_id: str, is_feature_variants: bool = True
) -> None:
"""
Represents an A/B test

variants_list = From the strategy document.
"""
self.variants = variants_list
self.feature_name = feature_name
self.group_id = group_id
self.is_feature_variants = is_feature_variants

def _apply_overrides(self, context: dict) -> dict:
"""
Expand Down Expand Up @@ -60,28 +63,31 @@ def _get_seed(context: dict, stickiness_selector: str = "default") -> str:
return seed

@staticmethod
def _format_variation(variation: dict) -> dict:
def _format_variation(variation: dict, flag_status: Optional[bool] = None) -> dict:
formatted_variation = copy.deepcopy(variation)
del formatted_variation["weight"]
if "overrides" in formatted_variation:
del formatted_variation["overrides"]
if "stickiness" in formatted_variation:
del formatted_variation["stickiness"]
if "enabled" not in formatted_variation and flag_status is not None:
formatted_variation["enabled"] = flag_status
return formatted_variation

def get_variant(self, context: dict) -> dict:
def get_variant(self, context: dict, flag_status: Optional[bool] = None) -> dict:
"""
Determines what variation a user is in.

:param context:
:param flag_status:
:return:
"""
fallback_variant = copy.deepcopy(DISABLED_VARIATION)

if self.variants:
override_variant = self._apply_overrides(context)
if override_variant:
return self._format_variation(override_variant)
return self._format_variation(override_variant, flag_status)

total_weight = sum(x["weight"] for x in self.variants)
if total_weight <= 0:
Expand All @@ -92,17 +98,18 @@ def get_variant(self, context: dict) -> dict:
if "stickiness" in self.variants[0].keys()
else "default"
)

target = utils.normalized_hash(
self._get_seed(context, stickiness_selector),
self.feature_name,
self.group_id,
total_weight,
)
counter = 0
for variation in self.variants:
counter += variation["weight"]

if counter >= target:
return self._format_variation(variation)
return self._format_variation(variation, flag_status)

# Catch all return.
return fallback_variant
43 changes: 28 additions & 15 deletions tests/specification_tests/test_client_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,43 @@ def load_specs():
return json.load(_f)


def get_client(state, test_context=None):
cache = FileCache("MOCK_CACHE")
cache.bootstrap_from_dict(state)
env = "default"
if test_context is not None and "environment" in test_context:
env = test_context["environment"]

unleash_client = UnleashClient(
url=URL,
app_name=APP_NAME,
instance_id="pytest_%s" % uuid.uuid4(),
disable_metrics=True,
disable_registration=True,
cache=cache,
environment=env,
)

unleash_client.initialize_client(fetch_toggles=False)
return unleash_client


def iter_spec():
for spec in load_specs():
name, state, tests, variant_tests = load_spec(spec)

cache = FileCache("MOCK_CACHE")
cache.bootstrap_from_dict(state)

unleash_client = UnleashClient(
url=URL,
app_name=APP_NAME,
instance_id="pytest_%s" % uuid.uuid4(),
disable_metrics=True,
disable_registration=True,
cache=cache,
)

unleash_client.initialize_client(fetch_toggles=False)
unleash_client = get_client(state=state)

for test in tests:
yield name, test["description"], unleash_client, test, False

for variant_test in variant_tests:
yield name, test["description"], unleash_client, variant_test, True
test_context = {}
if "context" in variant_test:
test_context = variant_test["context"]

unleash_client = get_client(state, test_context)
yield name, variant_test["description"], unleash_client, variant_test, True

unleash_client.destroy()

Expand Down Expand Up @@ -77,6 +91,5 @@ def test_spec(spec):
toggle_name = test_data["toggleName"]
expected = test_data["expectedResult"]
context = test_data.get("context")

variant = unleash_client.get_variant(toggle_name, context)
assert variant == expected
Loading