Skip to content

Commit

Permalink
refactor: rename get_feature_toggle to evaluate
Browse files Browse the repository at this point in the history
  • Loading branch information
heitorlessa committed Jul 30, 2021
1 parent a70bcd8 commit 649d987
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 46 deletions.
21 changes: 9 additions & 12 deletions aws_lambda_powertools/utilities/feature_toggles/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ def get_configuration(self) -> Dict[str, Any]:
self._schema_validator.validate_json_schema(config)
return config

def get_feature_toggle(
self, *, feature_name: str, context: Optional[Dict[str, Any]] = None, value_if_missing: bool
) -> bool:
def evaluate(self, *, feature_name: str, context: Optional[Dict[str, Any]] = None, default: bool) -> bool:
"""Get a feature toggle boolean value. Value is calculated according to a set of rules and conditions.
See below for explanation.
Expand All @@ -117,10 +115,9 @@ def get_feature_toggle(
context: Optional[Dict[str, Any]]
dict of attributes that you would like to match the rules
against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc.
value_if_missing: bool
this will be the returned value in case the feature toggle doesn't exist in
the schema or there has been an error while fetching the
configuration from appconfig
default: bool
default value if feature flag doesn't exist in the schema,
or there has been an error while fetching the configuration from appconfig
Returns
------
Expand All @@ -138,16 +135,16 @@ def get_feature_toggle(
try:
toggles_dict: Dict[str, Any] = self.get_configuration()
except ConfigurationError:
logger.error("unable to get feature toggles JSON, returning provided value_if_missing value")
return value_if_missing
logger.error("unable to get feature toggles JSON, returning provided default value")
return default

feature: Dict[str, Dict] = toggles_dict.get(schema.FEATURES_KEY, {}).get(feature_name, None)
if feature is None:
logger.warning(
f"feature does not appear in configuration, using provided value_if_missing, "
f"feature_name={feature_name}, value_if_missing={value_if_missing}"
f"feature does not appear in configuration, using provided default, "
f"feature_name={feature_name}, default={default}"
)
return value_if_missing
return default

rules_list = feature.get(schema.RULES_KEY)
feature_default_value = feature.get(schema.FEATURE_DEFAULT_VAL_KEY)
Expand Down
8 changes: 5 additions & 3 deletions docs/utilities/feature_flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ Potential changes to be validated when docs are in a better shape
- [x] ~~`rules_context` to `context`~~
- [x] `ConfigurationStore` to `FeatureFlags`
- [x] `StoreProvider` to `StoreProvider`
- [ ] Use `base.py` for interfaces for consistency (e.g. Metrics, Tracer, etc.)
- [x] ~~Use `base.py` for interfaces for consistency (e.g. Metrics, Tracer, etc.)~~
- [x] ~~AppConfig construct parameter names for consistency (e.g. `configuration_name` -> `name`, `service` -> `application`)~~
- [x] ~~Rename `value_if_missing` param to `default` to match Python consistency (e.g. `os.getenv("VAR", default=False)`)~~
- [x] ~~Review `get_feature` in favour of `evaluate`~~
- [ ] Some docstrings and logger refer to AWS AppConfig only (outdated given StoreProvider)
- [ ] Review why we're testing a private method(`is_rule_matched`)
- [x] AppConfig construct parameter names for consistency (e.g. `configuration_name` -> `name`, `service` -> `application`)
- [ ] Review `get_configuration`, `get_json_configuration`
- [ ] Review `get_feature` in favour of `get_feature_toggle`
- [ ] Review potentially redundant logging
52 changes: 21 additions & 31 deletions tests/functional/feature_toggles/test_feature_toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,18 @@ def test_toggles_rule_does_not_match(mocker, config):
}

conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(feature_name="my_feature", context={}, value_if_missing=False)
toggle = conf_store.evaluate(feature_name="my_feature", context={}, default=False)
assert toggle == expected_value


# this test checks that if you try to get a feature that doesn't exist in the schema,
# you get the default value of False that was sent to the get_feature_toggle API
# you get the default value of False that was sent to the evaluate API
def test_toggles_no_conditions_feature_does_not_exist(mocker, config):
expected_value = False
mocked_app_config_schema = {"features": {"my_fake_feature": {"feature_default_value": True}}}

conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(feature_name="my_feature", context={}, value_if_missing=expected_value)
toggle = conf_store.evaluate(feature_name="my_feature", context={}, default=expected_value)
assert toggle == expected_value


Expand All @@ -89,9 +89,7 @@ def test_toggles_no_rules(mocker, config):
expected_value = True
mocked_app_config_schema = {"features": {"my_feature": {"feature_default_value": expected_value}}}
conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, value_if_missing=False
)
toggle = conf_store.evaluate(feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False)
assert toggle == expected_value


Expand Down Expand Up @@ -119,9 +117,7 @@ def test_toggles_conditions_no_match(mocker, config):
},
}
conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, value_if_missing=False
)
toggle = conf_store.evaluate(feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False)
assert toggle == expected_value


Expand Down Expand Up @@ -156,13 +152,13 @@ def test_toggles_conditions_rule_match_equal_multiple_conditions(mocker, config)
},
}
conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(
toggle = conf_store.evaluate(
feature_name="my_feature",
context={
"tenant_id": tenant_id_val,
"username": username_val,
},
value_if_missing=True,
default=True,
)
assert toggle == expected_value

Expand Down Expand Up @@ -198,9 +194,7 @@ def test_toggles_conditions_no_rule_match_equal_multiple_conditions(mocker, conf
},
}
conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, value_if_missing=False
)
toggle = conf_store.evaluate(feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False)
assert toggle == expected_val


Expand Down Expand Up @@ -259,25 +253,25 @@ def test_toggles_conditions_rule_match_multiple_actions_multiple_rules_multiple_

conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
# match first rule
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "6", "username": "abcd"}, value_if_missing=False
toggle = conf_store.evaluate(
feature_name="my_feature", context={"tenant_id": "6", "username": "abcd"}, default=False
)
assert toggle == expected_value_first_check
# match second rule
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "4446", "username": "az"}, value_if_missing=False
toggle = conf_store.evaluate(
feature_name="my_feature", context={"tenant_id": "4446", "username": "az"}, default=False
)
assert toggle == expected_value_second_check
# match no rule
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "11114446", "username": "ab"}, value_if_missing=False
toggle = conf_store.evaluate(
feature_name="my_feature", context={"tenant_id": "11114446", "username": "ab"}, default=False
)
assert toggle == expected_value_third_check
# feature doesn't exist
toggle = conf_store.get_feature_toggle(
toggle = conf_store.evaluate(
feature_name="my_fake_feature",
context={"tenant_id": "11114446", "username": "ab"},
value_if_missing=expected_value_fourth_case,
default=expected_value_fourth_case,
)
assert toggle == expected_value_fourth_case

Expand Down Expand Up @@ -306,9 +300,7 @@ def test_toggles_match_rule_with_contains_action(mocker, config):
},
}
conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, value_if_missing=False
)
toggle = conf_store.evaluate(feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False)
assert toggle == expected_value


Expand All @@ -335,9 +327,7 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config):
},
}
conf_store = init_configuration_store(mocker, mocked_app_config_schema, config)
toggle = conf_store.get_feature_toggle(
feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, value_if_missing=False
)
toggle = conf_store.evaluate(feature_name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False)
assert toggle == expected_value


Expand Down Expand Up @@ -425,10 +415,10 @@ def test_get_feature_toggle_handles_error(mocker, config):
schema_fetcher = init_fetcher_side_effect(mocker, config, GetParameterError())
conf_store = FeatureFlags(schema_fetcher)

# WHEN calling get_feature_toggle
toggle = conf_store.get_feature_toggle(feature_name="Foo", value_if_missing=False)
# WHEN calling evaluate
toggle = conf_store.evaluate(feature_name="Foo", default=False)

# THEN handle the error and return the value_if_missing
# THEN handle the error and return the default
assert toggle is False


Expand Down

0 comments on commit 649d987

Please sign in to comment.