Skip to content

Commit ec84107

Browse files
author
Ran Isenberg
committed
fix(feature flags): fix handling multiple conditons where first condition matches and other dont
2 parents 8fa11f5 + 7b3b032 commit ec84107

File tree

7 files changed

+73
-21
lines changed

7 files changed

+73
-21
lines changed

.github/workflows/python_build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
- name: Complexity baseline
3838
run: make complexity-baseline
3939
- name: Upload coverage to Codecov
40-
uses: codecov/codecov-action@v2.0.1
40+
uses: codecov/codecov-action@v2.0.2
4141
with:
4242
file: ./coverage.xml
4343
# flags: unittests

.github/workflows/python_docs.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ on:
44
push:
55
branches:
66
- develop
7+
paths:
8+
- 'docs/**'
9+
- 'CHANGELOG.md'
10+
- 'mkdocs.yml'
711

812
jobs:
913
docs:

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ target:
55
@$(MAKE) pr
66

77
dev:
8-
pip install --upgrade pip pre-commit poetry==1.1.4
8+
pip install --upgrade pip pre-commit poetry
99
poetry install --extras "pydantic"
1010
pre-commit install
1111

aws_lambda_powertools/utilities/feature_flags/feature_flags.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ def _evaluate_conditions(
6464
rule_match_value = rule.get(schema.RULE_MATCH_VALUE)
6565
conditions = cast(List[Dict], rule.get(schema.CONDITIONS_KEY))
6666

67+
if not conditions:
68+
logger.debug(
69+
f"rule did not match, no conditions to match, rule_name={rule_name}, rule_value={rule_match_value}, "
70+
f"name={feature_name} "
71+
)
72+
return False
73+
6774
for condition in conditions:
6875
context_value = context.get(str(condition.get(schema.CONDITION_KEY)))
6976
cond_action = condition.get(schema.CONDITION_ACTION, "")
@@ -76,9 +83,8 @@ def _evaluate_conditions(
7683
)
7784
return False # context doesn't match condition
7885

79-
logger.debug(f"rule matched, rule_name={rule_name}, rule_value={rule_match_value}, name={feature_name}")
80-
return True
81-
return False
86+
logger.debug(f"rule matched, rule_name={rule_name}, rule_value={rule_match_value}, name={feature_name}")
87+
return True
8288

8389
def _evaluate_rules(
8490
self, *, feature_name: str, context: Dict[str, Any], feat_default: bool, rules: Dict[str, Any]
@@ -238,6 +244,7 @@ def get_enabled_features(self, *, context: Optional[Dict[str, Any]] = None) -> L
238244
logger.debug(f"Failed to fetch feature flags from store, returning empty list, reason={err}")
239245
return features_enabled
240246

247+
logger.debug("Evaluating all features")
241248
for name, feature in features.items():
242249
rules = feature.get(schema.RULES_KEY, {})
243250
feature_default_value = feature.get(schema.FEATURE_DEFAULT_VAL_KEY)

poetry.lock

Lines changed: 17 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ flake8-variables-names = "^0.0.4"
4242
isort = "^5.9.3"
4343
pytest-cov = "^2.12.1"
4444
pytest-mock = "^3.5.1"
45-
pdoc3 = "^0.9.2"
45+
pdoc3 = "^0.10.0"
4646
pytest-asyncio = "^0.15.1"
4747
bandit = "^1.7.0"
4848
radon = "^4.5.0"
4949
xenon = "^0.7.3"
5050
flake8-eradicate = "^1.1.0"
5151
flake8-bugbear = "^21.3.2"
52-
mkdocs-material = "^7.2.1"
52+
mkdocs-material = "^7.2.3"
5353
mkdocs-git-revision-date-plugin = "^0.3.1"
5454
mike = "^0.6.0"
5555
mypy = "^0.910"

tests/functional/feature_flags/test_feature_flags.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,44 @@ def test_flags_conditions_no_match(mocker, config):
121121

122122

123123
# check that a rule can match when it has multiple conditions, see rule name for further explanation
124+
def test_flags_conditions_rule_not_match_multiple_conditions_match_only_one_condition(mocker, config):
125+
expected_value = True
126+
tenant_id_val = "6"
127+
username_val = "a"
128+
mocked_app_config_schema = {
129+
"my_feature": {
130+
"default": True,
131+
"rules": {
132+
"tenant id equals 6 and username is a": {
133+
"when_match": False,
134+
"conditions": [
135+
{
136+
"action": RuleAction.EQUALS.value, # this condition matches
137+
"key": "tenant_id",
138+
"value": tenant_id_val,
139+
},
140+
{
141+
"action": RuleAction.EQUALS.value, # this condition does not
142+
"key": "username",
143+
"value": "bbb",
144+
},
145+
],
146+
}
147+
},
148+
}
149+
}
150+
feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
151+
toggle = feature_flags.evaluate(
152+
name="my_feature",
153+
context={
154+
"tenant_id": tenant_id_val,
155+
"username": username_val,
156+
},
157+
default=True,
158+
)
159+
assert toggle == expected_value
160+
161+
124162
def test_flags_conditions_rule_match_equal_multiple_conditions(mocker, config):
125163
expected_value = False
126164
tenant_id_val = "6"

0 commit comments

Comments
 (0)