From 029529a91b37f8249c3c5b7805470de1b3060fd2 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 26 Jun 2018 14:16:45 -0700 Subject: [PATCH 01/21] add Python versions to readme --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 606c11d1..5a806b2d 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,11 @@ LaunchDarkly SDK for Python [![Twitter Follow](https://img.shields.io/twitter/follow/launchdarkly.svg?style=social&label=Follow&maxAge=2592000)](https://twitter.com/intent/follow?screen_name=launchdarkly) +Supported Python versions +------------------------- + +This version of the LaunchDarkly SDK is compatible with Python 2.7, and Python 3.3 through 3.6. + Quick setup ----------- From ed20f3c37fbc1c8a6fc0e0ff1b8942b8b55dad2c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 28 Jun 2018 10:59:34 -0700 Subject: [PATCH 02/21] add Python 3.7-rc to build --- .circleci/config.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index e68a5ac8..4b730692 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -8,6 +8,7 @@ workflows: - test-3.4 - test-3.5 - test-3.6 + - test-3.7 test-template: &test-template steps: - checkout @@ -59,3 +60,8 @@ jobs: docker: - image: circleci/python:3.6-jessie - image: redis + test-3.6: + <<: *test-template + docker: + - image: circleci/python:3.7-rc-stretch + - image: redis From 53599b171e8a9c83bb52d076134f96f2a14f37cd Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 28 Jun 2018 11:16:06 -0700 Subject: [PATCH 03/21] fix config --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4b730692..c1178731 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -60,7 +60,7 @@ jobs: docker: - image: circleci/python:3.6-jessie - image: redis - test-3.6: + test-3.7: <<: *test-template docker: - image: circleci/python:3.7-rc-stretch From 8b8d87b8d6706667f5e900da3fd58a5a58e7c4b4 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 5 Jul 2018 10:47:32 -0700 Subject: [PATCH 04/21] use final 3.7 release in CI build --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c1178731..05cb973c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -63,5 +63,5 @@ jobs: test-3.7: <<: *test-template docker: - - image: circleci/python:3.7-rc-stretch + - image: circleci/python:3.7-stretch - image: redis From ca88418d8e455c04afd525634699094810cfd092 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 5 Jul 2018 14:14:45 -0700 Subject: [PATCH 05/21] update readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5a806b2d..57aac968 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ LaunchDarkly SDK for Python Supported Python versions ------------------------- -This version of the LaunchDarkly SDK is compatible with Python 2.7, and Python 3.3 through 3.6. +This version of the LaunchDarkly SDK is compatible with Python 2.7, and Python 3.3 through 3.7. Quick setup ----------- From ff280b295fd2ab8411b1ddbb1d607ce9255fc1d7 Mon Sep 17 00:00:00 2001 From: Andrew Shannon Brown Date: Mon, 23 Jul 2018 17:09:47 -0700 Subject: [PATCH 06/21] Remove @ashanbrown from CODEOWNERS --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index 44429ee1..8b137891 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @ashanbrown + From f0d757088c9668eabf2484047ef40e75d91ecd6d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 2 Aug 2018 16:36:05 -0700 Subject: [PATCH 07/21] better log output for stream failures --- ldclient/streaming.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ldclient/streaming.py b/ldclient/streaming.py index 89ef4faf..bac83433 100644 --- a/ldclient/streaming.py +++ b/ldclient/streaming.py @@ -5,6 +5,7 @@ from threading import Thread import backoff +import logging import time from ldclient.interfaces import UpdateProcessor @@ -32,6 +33,13 @@ def __init__(self, config, requester, store, ready): self._running = False self._ready = ready + # We need to suppress the default logging behavior of the backoff package, because + # it logs messages at ERROR level with variable content (the delay time) which will + # prevent monitors from coalescing multiple messages. The backoff package attempts + # to suppress its own output by default by giving the logger a NullHandler, but it + # will still propagate up to the root logger unless we do this: + logging.getLogger('backoff').propagate = False + # Retry/backoff logic: # Upon any error establishing the stream connection we retry with backoff + jitter. # Upon any error processing the results of the stream we reconnect after one second. @@ -65,8 +73,12 @@ def _backoff_expo(): def should_not_retry(e): return isinstance(e, UnsuccessfulResponseException) and (not is_http_error_recoverable(e.status)) + def log_backoff_message(props): + log.error("Streaming connection failed, will attempt to restart") + log.info("Will reconnect after delay of %fs", props['wait']) + @backoff.on_exception(_backoff_expo, BaseException, max_tries=None, jitter=backoff.full_jitter, - giveup=should_not_retry) + on_backoff=log_backoff_message, giveup=should_not_retry) def _connect(self): return SSEClient( self._uri, From 4d03e32e06ec7589cd91be4066b30d3d510082aa Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 17 Aug 2018 17:36:17 -0700 Subject: [PATCH 08/21] add new version of all_flags that captures more metadata --- ldclient/client.py | 66 ++++++++++++++------ ldclient/flags_state.py | 51 ++++++++++++++++ testing/test_flags_state.py | 47 +++++++++++++++ testing/test_ldclient.py | 22 ------- testing/test_ldclient_evaluation.py | 93 +++++++++++++++++++++++++++++ 5 files changed, 239 insertions(+), 40 deletions(-) create mode 100644 ldclient/flags_state.py create mode 100644 testing/test_flags_state.py create mode 100644 testing/test_ldclient_evaluation.py diff --git a/ldclient/client.py b/ldclient/client.py index 6c51e16f..56e20e6c 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -10,6 +10,7 @@ from ldclient.event_processor import NullEventProcessor from ldclient.feature_requester import FeatureRequesterImpl from ldclient.flag import evaluate +from ldclient.flags_state import FeatureFlagsState from ldclient.polling import PollingUpdateProcessor from ldclient.streaming import StreamingUpdateProcessor from ldclient.util import check_uwsgi, log @@ -199,33 +200,62 @@ def _evaluate_and_send_events(self, flag, user, default): return value def all_flags(self, user): - if self._config.offline: - log.warn("all_flags() called, but client is in offline mode. Returning None") + """Returns all feature flag values for the given user. + + This method is deprecated - please use all_flags_state instead. Current versions of the + client-side SDK (2.0.0 and later) will not generate analytics events correctly if you pass + the result of all_flags. + + :param user: the end user requesting the feature flags + :return a dictionary of feature flag keys to values; returns None if the client is offline, + has not been initialized, or the user is None or has no key + """ + state = self.all_flags_state(user) + if not state.valid: return None + return state.to_values_map() + + def all_flags_state(self, user): + """Returns an object that encapsulates the state of all feature flags for a given user, + including the flag values and also metadata that can be used on the front end. This method + does not send analytics events back to LaunchDarkly. + + :param user: the end user requesting the feature flags + :return a FeatureFlagsState object (will never be None; its 'valid' property will be False + if the client is offline, has not been initialized, or the user is None or has no key) + """ + if self._config.offline: + log.warn("all_flags_state() called, but client is in offline mode. Returning empty state") + return FeatureFlagsState(False) if not self.is_initialized(): if self._store.initialized: - log.warn("all_flags() called before client has finished initializing! Using last known values from feature store") + log.warn("all_flags_state() called before client has finished initializing! Using last known values from feature store") else: - log.warn("all_flags() called before client has finished initializing! Feature store unavailable - returning None") - return None + log.warn("all_flags_state() called before client has finished initializing! Feature store unavailable - returning empty state") + return FeatureFlagsState(False) if user is None or user.get('key') is None: - log.warn("User or user key is None when calling all_flags(). Returning None.") - return None - - def cb(all_flags): + log.warn("User or user key is None when calling all_flags_state(). Returning empty state.") + return FeatureFlagsState(False) + + state = FeatureFlagsState(True) + try: + flags_map = self._store.all(FEATURES, lambda x: x) + except Exception as e: + log.error("Unable to read flags for all_flag_state: %s" % e) + return FeatureFlagsState(False) + + for key, flag in flags_map.items(): try: - return self._evaluate_multi(user, all_flags) + result = self._evaluate(flag, user) + state.add_flag(flag, result.value, result.variation) except Exception as e: - log.error("Exception caught in all_flags: " + e.message + " for user: " + str(user)) - return {} - - return self._store.all(FEATURES, cb) - - def _evaluate_multi(self, user, flags): - return dict([(k, self._evaluate(v, user).value) for k, v in flags.items() or {}]) - + log.error("Error evaluating flag \"%s\" in all_flags_state: %s" % (key, e)) + state.add_flag(flag, None, None) + + return state + def secure_mode_hash(self, user): if user.get('key') is None or self._config.sdk_key is None: return "" diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py new file mode 100644 index 00000000..f2d094b3 --- /dev/null +++ b/ldclient/flags_state.py @@ -0,0 +1,51 @@ +import json + +class FeatureFlagsState(object): + """ + A snapshot of the state of all feature flags with regard to a specific user, generated by + calling the client's all_flags_state method. + """ + def __init__(self, valid): + self.__flag_values = {} + self.__flag_metadata = {} + self.__valid = valid + + def add_flag(self, flag, value, variation): + """Used internally to build the state map.""" + key = flag['key'] + self.__flag_values[key] = value + meta = { 'version': flag.get('version'), 'trackEvents': flag.get('trackEvents') } + if variation is not None: + meta['variation'] = variation + if flag.get('debugEventsUntilDate') is not None: + meta['debugEventsUntilDate'] = flag.get('debugEventsUntilDate') + self.__flag_metadata[key] = meta + + @property + def valid(self): + """True if this object contains a valid snapshot of feature flag state, or False if the + state could not be computed (for instance, because the client was offline or there was no user). + """ + return self.__valid + + def get_flag_value(self, key): + """Returns the value of an individual feature flag at the time the state was recorded. + :param string key: the feature flag key + :return: the flag's value; None if the flag returned the default value, or if there was no such flag + """ + return self.__flag_values.get(key) + + def to_values_map(self): + """Returns a dictionary of flag keys to flag values. If the flag would have evaluated to the + default value, its value will be None. + """ + return self.__flag_values + + def to_json_string(self): + """Returns a JSON string representation of the entire state map, in the format used by the + LaunchDarkly JavaScript SDK. Use this method if you are passing data to the front end that + will be used to "bootstrap" the JavaScript client. + """ + ret = self.__flag_values.copy() + ret['$flagsState'] = self.__flag_metadata + return json.dumps(ret) diff --git a/testing/test_flags_state.py b/testing/test_flags_state.py new file mode 100644 index 00000000..ff76683d --- /dev/null +++ b/testing/test_flags_state.py @@ -0,0 +1,47 @@ +import pytest +import json +from ldclient.flags_state import FeatureFlagsState + +def test_can_get_flag_value(): + state = FeatureFlagsState(True) + flag = { 'key': 'key' } + state.add_flag(flag, 'value', 1) + assert state.get_flag_value('key') == 'value' + +def test_returns_none_for_unknown_flag(): + state = FeatureFlagsState(True) + assert state.get_flag_value('key') is None + +def test_can_convert_to_values_map(): + state = FeatureFlagsState(True) + flag1 = { 'key': 'key1' } + flag2 = { 'key': 'key2' } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + assert state.to_values_map() == { 'key1': 'value1', 'key2': 'value2' } + +def test_can_convert_to_json_string(): + state = FeatureFlagsState(True) + flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } + flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + result = json.loads(state.to_json_string()) + assert result == { + 'key1': 'value1', + 'key2': 'value2', + '$flagsState': { + 'key1': { + 'variation': 0, + 'version': 100, + 'trackEvents': False + }, + 'key2': { + 'variation': 1, + 'version': 200, + 'trackEvents': True, + 'debugEventsUntilDate': 1000 + } + } + } diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index ce6ebdb5..db13a154 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -238,28 +238,6 @@ def test_event_for_existing_feature_with_no_user_key(): e['trackEvents'] == True) -def test_all_flags(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - } - } - store = InMemoryFeatureStore() - store.init({FEATURES: {'feature.key': feature}}) - client = LDClient(config=Config(sdk_key = 'SDK_KEY', - base_uri="http://localhost:3000", - event_processor_class=MockEventProcessor, - update_processor_class=MockUpdateProcessor, - feature_store=store)) - result = client.all_flags(user) - assert (len(result) == 1 and - result.get('feature.key') == 'b') - - def test_secure_mode_hash(): user = {'key': 'Message'} assert offline_client.secure_mode_hash(user) == "aa747c502a898200f9e4fa21bac68136f886a0e27aec70ba06daf2e2a5cb5597" diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py new file mode 100644 index 00000000..8ef7af83 --- /dev/null +++ b/testing/test_ldclient_evaluation.py @@ -0,0 +1,93 @@ +import pytest +import json +from ldclient.client import LDClient, Config +from ldclient.feature_store import InMemoryFeatureStore +from ldclient.versioned_data_kind import FEATURES +from testing.stub_util import MockEventProcessor, MockUpdateProcessor + + +user = { 'key': 'userkey' } +flag1 = { + 'key': 'key1', + 'version': 100, + 'on': False, + 'offVariation': 0, + 'variations': [ 'value1' ], + 'trackEvents': False +} +flag2 = { + 'key': 'key2', + 'version': 200, + 'on': False, + 'offVariation': 1, + 'variations': [ 'x', 'value2' ], + 'trackEvents': True, + 'debugEventsUntilDate': 1000 +} + +def make_client(store): + return LDClient(config=Config(sdk_key='SDK_KEY', + base_uri='http://test', + event_processor_class=MockEventProcessor, + update_processor_class=MockUpdateProcessor, + feature_store=store)) + +def test_all_flags_returns_values(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + result = client.all_flags(user) + assert result == { 'key1': 'value1', 'key2': 'value2' } + +def test_all_flags_returns_none_if_user_is_none(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + result = client.all_flags(None) + assert result is None + +def test_all_flags_returns_none_if_user_has_no_key(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + result = client.all_flags({ }) + assert result is None + +def test_all_flags_state_returns_state(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + state = client.all_flags_state(user) + assert state.valid == True + result = json.loads(state.to_json_string()) + assert result == { + 'key1': 'value1', + 'key2': 'value2', + '$flagsState': { + 'key1': { + 'variation': 0, + 'version': 100, + 'trackEvents': False + }, + 'key2': { + 'variation': 1, + 'version': 200, + 'trackEvents': True, + 'debugEventsUntilDate': 1000 + } + } + } + +def test_all_flags_state_returns_empty_state_if_user_is_none(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + state = client.all_flags_state(None) + assert state.valid == False + +def test_all_flags_state_returns_empty_state_if_user_has_no_key(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + state = client.all_flags_state({ }) + assert state.valid == False From f64fd292bea8cabd7cfac8b5731ffa26375de02b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 13:08:27 -0700 Subject: [PATCH 09/21] provide a method that returns a JSONable dictionary instead of just a string --- ldclient/flags_state.py | 25 +++++++++++++++++++------ testing/test_flags_state.py | 15 +++++++++++++-- testing/test_ldclient_evaluation.py | 2 +- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py index f2d094b3..c0dbdc2b 100644 --- a/ldclient/flags_state.py +++ b/ldclient/flags_state.py @@ -38,14 +38,27 @@ def get_flag_value(self, key): def to_values_map(self): """Returns a dictionary of flag keys to flag values. If the flag would have evaluated to the default value, its value will be None. + + Do not use this method if you are passing data to the front end to "bootstrap" the JavaScript client. + Instead, use to_json_dict. """ return self.__flag_values - - def to_json_string(self): - """Returns a JSON string representation of the entire state map, in the format used by the - LaunchDarkly JavaScript SDK. Use this method if you are passing data to the front end that - will be used to "bootstrap" the JavaScript client. + + def to_json_dict(self): + """Returns a dictionary suitable for passing as JSON, in the format used by the LaunchDarkly + JavaScript SDK. Use this method if you are passing data to the front end in order to + "bootstrap" the JavaScript client. """ ret = self.__flag_values.copy() ret['$flagsState'] = self.__flag_metadata - return json.dumps(ret) + return ret + + def to_json_string(self): + """Same as to_json_dict, but serializes the JSON structure into a string. + """ + return json.dumps(self.to_json_dict()) + + def __getstate__(self): + """Equivalent to to_json_dict() - used if you are serializing the object with jsonpickle. + """ + return self.to_json_dict() diff --git a/testing/test_flags_state.py b/testing/test_flags_state.py index ff76683d..19866075 100644 --- a/testing/test_flags_state.py +++ b/testing/test_flags_state.py @@ -20,14 +20,14 @@ def test_can_convert_to_values_map(): state.add_flag(flag2, 'value2', 1) assert state.to_values_map() == { 'key1': 'value1', 'key2': 'value2' } -def test_can_convert_to_json_string(): +def test_can_convert_to_json_dict(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } state.add_flag(flag1, 'value1', 0) state.add_flag(flag2, 'value2', 1) - result = json.loads(state.to_json_string()) + result = state.to_json_dict() assert result == { 'key1': 'value1', 'key2': 'value2', @@ -45,3 +45,14 @@ def test_can_convert_to_json_string(): } } } + +def test_can_convert_to_json_string(): + state = FeatureFlagsState(True) + flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } + flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + obj = state.to_json_dict() + str = state.to_json_string() + assert json.loads(str) == obj diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index 8ef7af83..3cdc6c59 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -59,7 +59,7 @@ def test_all_flags_state_returns_state(): client = make_client(store) state = client.all_flags_state(user) assert state.valid == True - result = json.loads(state.to_json_string()) + result = state.to_json_dict() assert result == { 'key1': 'value1', 'key2': 'value2', From f6e019a77756cea7d8a9c7125b0397f10dc60b82 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 20:17:06 -0700 Subject: [PATCH 10/21] misc fixes --- ldclient/flags_state.py | 5 ++++- testing/test_flags_state.py | 15 ++++++++++++++- testing/test_ldclient_evaluation.py | 3 ++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py index c0dbdc2b..7e8ab3b9 100644 --- a/ldclient/flags_state.py +++ b/ldclient/flags_state.py @@ -3,7 +3,9 @@ class FeatureFlagsState(object): """ A snapshot of the state of all feature flags with regard to a specific user, generated by - calling the client's all_flags_state method. + calling the client's all_flags_state method. Serializing this object to JSON, using the + to_json_dict method or jsonpickle, will produce the appropriate data structure for + bootstrapping the LaunchDarkly JavaScript client. """ def __init__(self, valid): self.__flag_values = {} @@ -51,6 +53,7 @@ def to_json_dict(self): """ ret = self.__flag_values.copy() ret['$flagsState'] = self.__flag_metadata + ret['$valid'] = self.__valid return ret def to_json_string(self): diff --git a/testing/test_flags_state.py b/testing/test_flags_state.py index 19866075..c948dd3d 100644 --- a/testing/test_flags_state.py +++ b/testing/test_flags_state.py @@ -1,5 +1,6 @@ import pytest import json +import jsonpickle from ldclient.flags_state import FeatureFlagsState def test_can_get_flag_value(): @@ -43,7 +44,8 @@ def test_can_convert_to_json_dict(): 'trackEvents': True, 'debugEventsUntilDate': 1000 } - } + }, + '$valid': True } def test_can_convert_to_json_string(): @@ -56,3 +58,14 @@ def test_can_convert_to_json_string(): obj = state.to_json_dict() str = state.to_json_string() assert json.loads(str) == obj + +def test_can_serialize_with_jsonpickle(): + state = FeatureFlagsState(True) + flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } + flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + obj = state.to_json_dict() + str = jsonpickle.encode(state, unpicklable=False) + assert json.loads(str) == obj diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index 3cdc6c59..92187c17 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -75,7 +75,8 @@ def test_all_flags_state_returns_state(): 'trackEvents': True, 'debugEventsUntilDate': 1000 } - } + }, + '$valid': True } def test_all_flags_state_returns_empty_state_if_user_is_none(): From 3b9efb6e949e5b0f5f39faa07c396905062f7d6e Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 21 Aug 2018 12:37:32 -0700 Subject: [PATCH 11/21] add ability to filter for only client-side flags --- ldclient/client.py | 8 +++++- testing/test_ldclient_evaluation.py | 39 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/ldclient/client.py b/ldclient/client.py index 56e20e6c..51167025 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -215,12 +215,15 @@ def all_flags(self, user): return None return state.to_values_map() - def all_flags_state(self, user): + def all_flags_state(self, user, **kwargs): """Returns an object that encapsulates the state of all feature flags for a given user, including the flag values and also metadata that can be used on the front end. This method does not send analytics events back to LaunchDarkly. :param user: the end user requesting the feature flags + :param kwargs: optional parameters affecting how the state is computed: set + `client_side_only=True` to limit it to only flags that are marked for use with the + client-side SDK (by default, all flags are included) :return a FeatureFlagsState object (will never be None; its 'valid' property will be False if the client is offline, has not been initialized, or the user is None or has no key) """ @@ -240,6 +243,7 @@ def all_flags_state(self, user): return FeatureFlagsState(False) state = FeatureFlagsState(True) + client_only = kwargs.get('client_side_only', False) try: flags_map = self._store.all(FEATURES, lambda x: x) except Exception as e: @@ -247,6 +251,8 @@ def all_flags_state(self, user): return FeatureFlagsState(False) for key, flag in flags_map.items(): + if client_only and not flag.get('clientSide', False): + continue try: result = self._evaluate(flag, user) state.add_flag(flag, result.value, result.variation) diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index 92187c17..be8c74c5 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -79,6 +79,45 @@ def test_all_flags_state_returns_state(): '$valid': True } +def test_all_flags_state_can_be_filtered_for_client_side_flags(): + flag1 = { + 'key': 'server-side-1', + 'on': False, + 'offVariation': 0, + 'variations': [ 'a' ], + 'clientSide': False + } + flag2 = { + 'key': 'server-side-2', + 'on': False, + 'offVariation': 0, + 'variations': [ 'b' ], + 'clientSide': False + } + flag3 = { + 'key': 'client-side-1', + 'on': False, + 'offVariation': 0, + 'variations': [ 'value1' ], + 'clientSide': True + } + flag4 = { + 'key': 'client-side-2', + 'on': False, + 'offVariation': 0, + 'variations': [ 'value2' ], + 'clientSide': True + } + + store = InMemoryFeatureStore() + store.init({ FEATURES: { flag1['key']: flag1, flag2['key']: flag2, flag3['key']: flag3, flag4['key']: flag4 } }) + client = make_client(store) + + state = client.all_flags_state(user, client_side_only=True) + assert state.valid == True + values = state.to_values_map() + assert values == { 'client-side-1': 'value1', 'client-side-2': 'value2' } + def test_all_flags_state_returns_empty_state_if_user_is_none(): store = InMemoryFeatureStore() store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) From 5523d2d35e2787b52ef00d9849424dbff7168a2f Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 23 Aug 2018 12:44:00 -0700 Subject: [PATCH 12/21] implement evaluation with explanations --- ldclient/client.py | 143 ++++++++++++++--------- ldclient/event_processor.py | 2 + ldclient/flag.py | 170 ++++++++++++++++++++-------- ldclient/flags_state.py | 13 ++- testing/test_flag.py | 169 +++++++++++++++++++++------ testing/test_flags_state.py | 18 +-- testing/test_ldclient.py | 110 ++++++++++-------- testing/test_ldclient_evaluation.py | 86 ++++++++++++++ 8 files changed, 515 insertions(+), 196 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 51167025..32c054c0 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -3,13 +3,14 @@ import hashlib import hmac import threading +import traceback from builtins import object from ldclient.config import Config as Config from ldclient.event_processor import NullEventProcessor from ldclient.feature_requester import FeatureRequesterImpl -from ldclient.flag import evaluate +from ldclient.flag import EvaluationDetail, evaluate, error_reason from ldclient.flags_state import FeatureFlagsState from ldclient.polling import PollingUpdateProcessor from ldclient.streaming import StreamingUpdateProcessor @@ -136,17 +137,60 @@ def toggle(self, key, user, default): return self.variation(key, user, default) def variation(self, key, user, default): + """ + Determines the variation of a feature flag for a user. + + :param string key: the unique key for the feature flag + :param dict user: a dictionary containing parameters for the end user requesting the flag + :param object default: the default value of the flag, to be used if the value is not + available from LaunchDarkly + :return: one of the flag's variation values, or the default value + """ + return self._evaluate_internal(key, user, default, False).value + + def variation_detail(self, key, user, default): + """ + Determines the variation of a feature flag for a user, like `variation`, but also + provides additional information about how this value was calculated. + + The return value is an EvaluationDetail object, which has three properties: + + `value`: the value that was calculated for this user (same as the return value + of `variation`) + + `variation_index`: the positional index of this value in the flag, e.g. 0 for the + first variation - or `None` if the default value was returned + + `reason`: a hash describing the main reason why this value was selected. + + The `reason` will also be included in analytics events, if you are capturing + detailed event data for this flag. + + :param string key: the unique key for the feature flag + :param dict user: a dictionary containing parameters for the end user requesting the flag + :param object default: the default value of the flag, to be used if the value is not + available from LaunchDarkly + :return: an EvaluationDetail object describing the result + :rtype: EvaluationDetail + """ + return self._evaluate_internal(key, user, default, True) + + def _evaluate_internal(self, key, user, default, include_reasons_in_events): default = self._config.get_default(key, default) - if user is not None: - self._sanitize_user(user) if self._config.offline: - return default + return EvaluationDetail(default, None, error_reason('CLIENT_NOT_READY')) + + if user is not None: + self._sanitize_user(user) - def send_event(value, version=None): - self._send_event({'kind': 'feature', 'key': key, 'user': user, 'variation': None, - 'value': value, 'default': default, 'version': version, - 'trackEvents': False, 'debugEventsUntilDate': None}) + def send_event(value, variation=None, flag=None, reason=None): + self._send_event({'kind': 'feature', 'key': key, 'user': user, + 'value': value, 'variation': variation, 'default': default, + 'version': flag.get('version') if flag else None, + 'trackEvents': flag.get('trackEvents') if flag else None, + 'debugEventsUntilDate': flag.get('debugEventsUntilDate') if flag else None, + 'reason': reason if include_reasons_in_events else None}) if not self.is_initialized(): if self._store.initialized: @@ -154,57 +198,43 @@ def send_event(value, version=None): else: log.warn("Feature Flag evaluation attempted before client has initialized! Feature store unavailable - returning default: " + str(default) + " for feature key: " + key) - send_event(default) - return default - + reason = error_reason('CLIENT_NOT_READY') + send_event(default, None, None, reason) + return EvaluationDetail(default, None, reason) + if user is not None and user.get('key', "") == "": log.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly.") - def cb(flag): - try: - if not flag: - log.info("Feature Flag key: " + key + " not found in Feature Store. Returning default.") - send_event(default) - return default - - return self._evaluate_and_send_events(flag, user, default) - - except Exception as e: - log.error("Exception caught in variation: " + e.message + " for flag key: " + key + " and user: " + str(user)) - send_event(default) - - return default - - return self._store.get(FEATURES, key, cb) - - def _evaluate(self, flag, user): - return evaluate(flag, user, self._store) - - def _evaluate_and_send_events(self, flag, user, default): - if user is None or user.get('key') is None: - log.warn("Missing user or user key when evaluating Feature Flag key: " + flag.get('key') + ". Returning default.") - value = default - variation = None + flag = self._store.get(FEATURES, key, lambda x: x) + if not flag: + reason = error_reason('FLAG_NOT_FOUND') + send_event(default, None, None, reason) + return EvaluationDetail(default, None, reason) else: - result = evaluate(flag, user, self._store) - for event in result.events or []: - self._send_event(event) - value = default if result.value is None else result.value - variation = result.variation - - self._send_event({'kind': 'feature', 'key': flag.get('key'), - 'user': user, 'variation': variation, 'value': value, - 'default': default, 'version': flag.get('version'), - 'trackEvents': flag.get('trackEvents'), - 'debugEventsUntilDate': flag.get('debugEventsUntilDate')}) - return value + if user is None or user.get('key') is None: + reason = error_reason('USER_NOT_SPECIFIED') + send_event(default, None, flag, reason) + return EvaluationDetail(default, None, reason) + try: + result = evaluate(flag, user, self._store, include_reasons_in_events) + for event in result.events or []: + self._send_event(event) + value = default if result.detail.variation_index is None else result.detail.value + send_event(value, result.detail.variation_index, flag, result.detail.reason) + return EvaluationDetail(value, result.detail.variation_index, result.detail.reason) + except Exception as e: + log.error("Unexpected error while evaluating feature flag \"%s\": %s" % (key, e)) + log.debug(traceback.format_exc()) + reason = error_reason('EXCEPTION') + send_event(default, None, flag, reason) + return EvaluationDetail(default, None, reason) + def all_flags(self, user): """Returns all feature flag values for the given user. This method is deprecated - please use all_flags_state instead. Current versions of the - client-side SDK (2.0.0 and later) will not generate analytics events correctly if you pass - the result of all_flags. + client-side SDK will not generate analytics events correctly if you pass the result of all_flags. :param user: the end user requesting the feature flags :return a dictionary of feature flag keys to values; returns None if the client is offline, @@ -223,7 +253,8 @@ def all_flags_state(self, user, **kwargs): :param user: the end user requesting the feature flags :param kwargs: optional parameters affecting how the state is computed: set `client_side_only=True` to limit it to only flags that are marked for use with the - client-side SDK (by default, all flags are included) + client-side SDK (by default, all flags are included); set `with_reasons=True` to + include evaluation reasons in the state (see `variation_detail`) :return a FeatureFlagsState object (will never be None; its 'valid' property will be False if the client is offline, has not been initialized, or the user is None or has no key) """ @@ -244,6 +275,7 @@ def all_flags_state(self, user, **kwargs): state = FeatureFlagsState(True) client_only = kwargs.get('client_side_only', False) + with_reasons = kwargs.get('with_reasons', False) try: flags_map = self._store.all(FEATURES, lambda x: x) except Exception as e: @@ -254,11 +286,14 @@ def all_flags_state(self, user, **kwargs): if client_only and not flag.get('clientSide', False): continue try: - result = self._evaluate(flag, user) - state.add_flag(flag, result.value, result.variation) + detail = evaluate(flag, user, self._store, False).detail + state.add_flag(flag, detail.value, detail.variation_index, + detail.reason if with_reasons else None) except Exception as e: log.error("Error evaluating flag \"%s\" in all_flags_state: %s" % (key, e)) - state.add_flag(flag, None, None) + log.debug(traceback.format_exc()) + reason = {'kind': 'ERROR', 'errorKind': 'EXCEPTION'} + state.add_flag(flag, None, None, reason if with_reasons else None) return state diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index fa6061b4..3b89420f 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -84,6 +84,8 @@ def make_output_event(self, e): out['user'] = self._user_filter.filter_user_props(e['user']) else: out['userKey'] = e['user'].get('key') + if e.get('reason'): + out['reason'] = e.get('reason') return out elif kind == 'identify': return { diff --git a/ldclient/flag.py b/ldclient/flag.py index 7b0e9ed3..3956e956 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -16,26 +16,107 @@ log = logging.getLogger(sys.modules[__name__].__name__) -EvalResult = namedtuple('EvalResult', ['variation', 'value', 'events']) +class EvaluationDetail(object): + """ + The return type of LDClient.variation_detail, combining the result of a flag evaluation + with information about how it was calculated. + """ + def __init__(self, value, variation_index, reason): + self.__value = value + self.__variation_index = variation_index + self.__reason = reason + + @property + def value(self): + """The result of the flag evaluation. This will be either one of the flag's + variations or the default value that was passed to the variation() method. + """ + return self.__value + + @property + def variation_index(self): + """The index of the returned value within the flag's list of variations, e.g. + 0 for the first variation - or None if the default value was returned. + """ + return self.__variation_index + + @property + def reason(self): + """A dictionary describing the main factor that influenced the flag evaluation value. + It contains the following properties: + + 'kind': The general category of reason, as follows: 'OFF' - the flag was off; + 'FALLTHROUGH' - the flag was on but the user did not match any targets or rules; + 'TARGET_MATCH' - the user was specifically targeted for this flag; 'RULE_MATCH' - + the user matched one of the flag's rules; 'PREREQUISITE_FAILED' - the flag was + considered off because it had at least one prerequisite flag that did not return + the desired variation; 'ERROR' - the flag could not be evaluated due to an + unexpected error. + + 'ruleIndex', 'ruleId': The positional index and unique identifier of the matched + rule, if the kind was 'RULE_MATCH' + + 'prerequisiteKey': The flag key of the prerequisite that failed, if the kind was + 'PREREQUISITE_FAILED' + + 'errorKind': further describes the nature of the error if the kind was 'ERROR', + e.g. 'FLAG_NOT_FOUND' + """ + return self.__reason + + def __eq__(self, other): + return self.value == other.value and self.variation_index == other.variation_index and self.reason == other.reason + + def __ne__(self, other): + return not self.__eq__(other) + + def __str__(self): + return "(value=%s, variation_index=%s, reason=%s)" % (self.value, self.variation_index, self.reason) + + def __repr__(self): + return self.__str__() + + +EvalResult = namedtuple('EvalResult', ['detail', 'events']) + + +def error_reason(error_kind): + return {'kind': 'ERROR', 'errorKind': error_kind} + + +def evaluate(flag, user, store, include_reasons_in_events = False): + if flag.get('on', False): + prereq_events = [] + detail = _evaluate(flag, user, store, prereq_events, include_reasons_in_events) + return EvalResult(detail = detail, events = prereq_events) + return EvalResult(detail = _get_off_value(flag, {'kind': 'OFF'}), events = []) -def evaluate(flag, user, store): - prereq_events = [] - if flag.get('on', False): - variation, value, prereq_events = _evaluate(flag, user, store) - if value is not None: - return EvalResult(variation = variation, value = value, events = prereq_events) +def _evaluate(flag, user, store, prereq_events, include_reasons_in_events): + prereq_failure_reason = _check_prerequisites(flag, user, store, prereq_events, include_reasons_in_events) + if prereq_failure_reason is not None: + return _get_off_value(flag, prereq_failure_reason) - off_var = flag.get('offVariation') - off_value = None if off_var is None else _get_variation(flag, off_var) - return EvalResult(variation = off_var, value = off_value, events = prereq_events) + # Check to see if any user targets match: + for target in flag.get('targets') or []: + for value in target.get('values') or []: + if value == user['key']: + return _get_variation(flag, target.get('variation'), {'kind': 'TARGET_MATCH'}) + # Now walk through the rules to see if any match + for index, rule in enumerate(flag.get('rules') or []): + if _rule_matches_user(rule, user, store): + return _get_value_for_variation_or_rollout(flag, rule, user, + {'kind': 'RULE_MATCH', 'ruleIndex': index, 'ruleId': rule.get('id')}) + + # Walk through fallthrough and see if it matches + if flag.get('fallthrough') is not None: + return _get_value_for_variation_or_rollout(flag, flag['fallthrough'], user, {'kind': 'FALLTHROUGH'}) -def _evaluate(flag, user, store, prereq_events=None): - events = prereq_events or [] + +def _check_prerequisites(flag, user, store, events, include_reasons_in_events): failed_prereq = None - prereq_var = None - prereq_value = None + prereq_res = None for prereq in flag.get('prerequisites') or []: prereq_flag = store.get(FEATURES, prereq.get('key'), lambda x: x) if prereq_flag is None: @@ -43,54 +124,45 @@ def _evaluate(flag, user, store, prereq_events=None): failed_prereq = prereq break if prereq_flag.get('on', False) is True: - prereq_var, prereq_value, events = _evaluate(prereq_flag, user, store, events) - if prereq_var is None or not prereq_var == prereq.get('variation'): + prereq_res = _evaluate(prereq_flag, user, store, events, include_reasons_in_events) + if prereq_res.variation_index != prereq.get('variation'): failed_prereq = prereq else: failed_prereq = prereq - event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, 'variation': prereq_var, - 'value': prereq_value, 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), + event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, + 'variation': prereq_res.variation_index if prereq_res else None, + 'value': prereq_res.value if prereq_res else None, + 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), 'trackEvents': prereq_flag.get('trackEvents'), - 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate')} + 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate'), + 'reason': prereq_res.reason if prereq_res and include_reasons_in_events else None} events.append(event) - if failed_prereq is not None: - return None, None, events - - index = _evaluate_index(flag, user, store) - return index, _get_variation(flag, index), events - - -def _evaluate_index(feature, user, store): - # Check to see if any user targets match: - for target in feature.get('targets') or []: - for value in target.get('values') or []: - if value == user['key']: - return target.get('variation') - - # Now walk through the rules to see if any match - for rule in feature.get('rules') or []: - if _rule_matches_user(rule, user, store): - return _variation_index_for_user(feature, rule, user) + if failed_prereq: + return {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': failed_prereq.get('key')} + return None - # Walk through fallthrough and see if it matches - if feature.get('fallthrough') is not None: - return _variation_index_for_user(feature, feature['fallthrough'], user) - return None +def _get_variation(flag, variation, reason): + vars = flag.get('variations') or [] + if variation < 0 or variation >= len(vars): + return EvaluationDetail(None, None, error_reason('MALFORMED_FLAG')) + return EvaluationDetail(vars[variation], variation, reason) -def _get_variation(feature, index): - if index is not None and index < len(feature['variations']): - return feature['variations'][index] - return None +def _get_off_value(flag, reason): + off_var = flag.get('offVariation') + if off_var is None: + return EvaluationDetail(None, None, reason) + return _get_variation(flag, off_var, reason) -def _get_off_variation(feature): - if feature.get('offVariation') is not None: - return _get_variation(feature, feature.get('offVariation')) - return None +def _get_value_for_variation_or_rollout(flag, vr, user, reason): + index = _variation_index_for_user(flag, vr, user) + if index is None: + return EvaluationDetail(None, None, error_reason('MALFORMED_FLAG')) + return _get_variation(flag, index, reason) def _get_user_attribute(user, attr): diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py index 7e8ab3b9..c76b4908 100644 --- a/ldclient/flags_state.py +++ b/ldclient/flags_state.py @@ -12,13 +12,15 @@ def __init__(self, valid): self.__flag_metadata = {} self.__valid = valid - def add_flag(self, flag, value, variation): + def add_flag(self, flag, value, variation, reason): """Used internally to build the state map.""" key = flag['key'] self.__flag_values[key] = value meta = { 'version': flag.get('version'), 'trackEvents': flag.get('trackEvents') } if variation is not None: meta['variation'] = variation + if reason is not None: + meta['reason'] = reason if flag.get('debugEventsUntilDate') is not None: meta['debugEventsUntilDate'] = flag.get('debugEventsUntilDate') self.__flag_metadata[key] = meta @@ -37,6 +39,15 @@ def get_flag_value(self, key): """ return self.__flag_values.get(key) + def get_flag_reason(self, key): + """Returns the evaluation reason for an individual feature flag at the time the state was recorded. + :param string key: the feature flag key + :return: a dictionary describing the reason; None if reasons were not recorded, or if there was no + such flag + """ + meta = self.__flag_metadata.get(key) + return None if meta is None else meta.get('reason') + def to_values_map(self): """Returns a dictionary of flag keys to flag values. If the flag would have evaluated to the default value, its value will be None. diff --git a/testing/test_flag.py b/testing/test_flag.py index 29d2bb61..fbe54939 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -1,32 +1,65 @@ import pytest from ldclient.feature_store import InMemoryFeatureStore -from ldclient.flag import EvalResult, _bucket_user, evaluate +from ldclient.flag import EvaluationDetail, EvalResult, _bucket_user, evaluate from ldclient.versioned_data_kind import FEATURES, SEGMENTS empty_store = InMemoryFeatureStore() +def make_boolean_flag_with_rules(rules): + return { + 'key': 'feature', + 'on': True, + 'rules': rules, + 'fallthrough': { 'variation': 0 }, + 'variations': [ False, True ], + 'salt': '' + } + + def test_flag_returns_off_variation_if_flag_is_off(): flag = { 'key': 'feature', 'on': False, 'offVariation': 1, - 'fallthrough': { 'variation': 0 }, 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == EvalResult(1, 'b', []) + detail = EvaluationDetail('b', 1, {'kind': 'OFF'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_flag_returns_none_if_flag_is_off_and_off_variation_is_unspecified(): flag = { 'key': 'feature', 'on': False, - 'fallthrough': { 'variation': 0 }, 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == EvalResult(None, None, []) + detail = EvaluationDetail(None, None, {'kind': 'OFF'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_off_variation_is_too_high(): + flag = { + 'key': 'feature', + 'on': False, + 'offVariation': 999, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_off_variation_is_negative(): + flag = { + 'key': 'feature', + 'on': False, + 'offVariation': -1, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_flag_returns_off_variation_if_prerequisite_not_found(): flag = { @@ -38,7 +71,8 @@ def test_flag_returns_off_variation_if_prerequisite_not_found(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == EvalResult(1, 'b', []) + detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'badfeature'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): store = InMemoryFeatureStore() @@ -61,9 +95,10 @@ def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): } store.upsert(FEATURES, flag1) user = { 'key': 'x' } + detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'feature1'}) events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 0, 'value': 'd', - 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] - assert evaluate(flag, user, store) == EvalResult(1, 'b', events_should_be) + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None, 'reason': None}] + assert evaluate(flag, user, store) == EvalResult(detail, events_should_be) def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_rules(): store = InMemoryFeatureStore() @@ -86,44 +121,104 @@ def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_ru } store.upsert(FEATURES, flag1) user = { 'key': 'x' } + detail = EvaluationDetail('a', 0, {'kind': 'FALLTHROUGH'}) events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', - 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] - assert evaluate(flag, user, store) == EvalResult(0, 'a', events_should_be) + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None, 'reason': None}] + assert evaluate(flag, user, store) == EvalResult(detail, events_should_be) -def test_flag_matches_user_from_targets(): +def test_flag_returns_error_if_fallthrough_variation_is_too_high(): flag = { - 'key': 'feature0', + 'key': 'feature', 'on': True, - 'targets': [{ 'values': ['whoever', 'userkey'], 'variation': 2 }], - 'fallthrough': { 'variation': 0 }, - 'offVariation': 1, + 'fallthrough': {'variation': 999}, 'variations': ['a', 'b', 'c'] } - user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == EvalResult(2, 'c', []) + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) -def test_flag_matches_user_from_rules(): +def test_flag_returns_error_if_fallthrough_variation_is_negative(): + flag = { + 'key': 'feature', + 'on': True, + 'fallthrough': {'variation': -1}, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_fallthrough_has_no_variation_or_rollout(): + flag = { + 'key': 'feature', + 'on': True, + 'fallthrough': {}, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_fallthrough_has_rollout_with_no_variations(): + flag = { + 'key': 'feature', + 'on': True, + 'fallthrough': {'rollout': {'variations': []}}, + 'variations': ['a', 'b', 'c'], + 'salt': '' + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_matches_user_from_targets(): flag = { 'key': 'feature0', 'on': True, - 'rules': [ - { - 'clauses': [ - { - 'attribute': 'key', - 'op': 'in', - 'values': [ 'userkey' ] - } - ], - 'variation': 2 - } - ], + 'targets': [{ 'values': ['whoever', 'userkey'], 'variation': 2 }], 'fallthrough': { 'variation': 0 }, 'offVariation': 1, 'variations': ['a', 'b', 'c'] } user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == EvalResult(2, 'c', []) + detail = EvaluationDetail('c', 2, {'kind': 'TARGET_MATCH'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_matches_user_from_rules(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], 'variation': 1} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(True, 1, {'kind': 'RULE_MATCH', 'ruleIndex': 0, 'ruleId': 'id'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_variation_is_too_high(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], 'variation': 999} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_variation_is_negative(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], 'variation': -1} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_has_no_variation_or_rollout(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}]} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_has_rollout_with_no_variations(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], + 'rollout': {'variations': []} } + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_segment_match_clause_retrieves_segment_from_store(): store = InMemoryFeatureStore() @@ -154,7 +249,7 @@ def test_segment_match_clause_retrieves_segment_from_store(): ] } - assert evaluate(flag, user, store) == EvalResult(1, True, []) + assert evaluate(flag, user, store).detail.value == True def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found(): user = { "key": "foo" } @@ -177,7 +272,7 @@ def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found( ] } - assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) + assert evaluate(flag, user, empty_store).detail.value == False def test_clause_matches_builtin_attribute(): clause = { @@ -187,7 +282,7 @@ def test_clause_matches_builtin_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(1, True, []) + assert evaluate(flag, user, empty_store).detail.value == True def test_clause_matches_custom_attribute(): clause = { @@ -197,7 +292,7 @@ def test_clause_matches_custom_attribute(): } user = { 'key': 'x', 'name': 'Bob', 'custom': { 'legs': 4 } } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(1, True, []) + assert evaluate(flag, user, empty_store).detail.value == True def test_clause_returns_false_for_missing_attribute(): clause = { @@ -207,7 +302,7 @@ def test_clause_returns_false_for_missing_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) + assert evaluate(flag, user, empty_store).detail.value == False def test_clause_can_be_negated(): clause = { @@ -218,7 +313,7 @@ def test_clause_can_be_negated(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) + assert evaluate(flag, user, empty_store).detail.value == False def _make_bool_flag_from_clause(clause): diff --git a/testing/test_flags_state.py b/testing/test_flags_state.py index c948dd3d..2fe5b123 100644 --- a/testing/test_flags_state.py +++ b/testing/test_flags_state.py @@ -6,7 +6,7 @@ def test_can_get_flag_value(): state = FeatureFlagsState(True) flag = { 'key': 'key' } - state.add_flag(flag, 'value', 1) + state.add_flag(flag, 'value', 1, None) assert state.get_flag_value('key') == 'value' def test_returns_none_for_unknown_flag(): @@ -17,16 +17,16 @@ def test_can_convert_to_values_map(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1' } flag2 = { 'key': 'key2' } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) assert state.to_values_map() == { 'key1': 'value1', 'key2': 'value2' } def test_can_convert_to_json_dict(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) result = state.to_json_dict() assert result == { @@ -52,8 +52,8 @@ def test_can_convert_to_json_string(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) obj = state.to_json_dict() str = state.to_json_string() @@ -63,8 +63,8 @@ def test_can_serialize_with_jsonpickle(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) obj = state.to_json_dict() str = jsonpickle.encode(state, unpicklable=False) diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index db13a154..1766386b 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -58,6 +58,17 @@ def make_client(store): feature_store=store)) +def make_off_flag_with_value(key, value): + return { + u'key': key, + u'version': 100, + u'salt': u'', + u'on': False, + u'variations': [value], + u'offVariation': 0 + } + + def get_first_event(c): return c._event_processor._events.pop(0) @@ -149,93 +160,100 @@ def test_no_defaults(): def test_event_for_existing_feature(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - }, - u'trackEvents': True - } + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) client = make_client(store) - assert 'b' == client.variation('feature.key', user, default='c') + assert 'value' == client.variation('feature.key', user, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == user and - e['value'] == 'b' and - e['variation'] == 1 and - e['default'] == 'c' and - e['trackEvents'] == True) + e['version'] == feature['version'] and + e['value'] == 'value' and + e['variation'] == 0 and + e.get('reason') is None and + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) + + +def test_event_for_existing_feature_with_reason(): + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'value' == client.variation_detail('feature.key', user, default='default').value + e = get_first_event(client) + assert (e['kind'] == 'feature' and + e['key'] == 'feature.key' and + e['user'] == user and + e['version'] == feature['version'] and + e['value'] == 'value' and + e['variation'] == 0 and + e['reason'] == {'kind': 'OFF'} and + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) def test_event_for_unknown_feature(): store = InMemoryFeatureStore() store.init({FEATURES: {}}) client = make_client(store) - assert 'c' == client.variation('feature.key', user, default='c') + assert 'default' == client.variation('feature.key', user, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == user and - e['value'] == 'c' and + e['value'] == 'default' and e['variation'] == None and - e['default'] == 'c') + e['default'] == 'default') def test_event_for_existing_feature_with_no_user(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - }, - u'trackEvents': True - } + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) client = make_client(store) - assert 'c' == client.variation('feature.key', None, default='c') + assert 'default' == client.variation('feature.key', None, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == None and - e['value'] == 'c' and + e['version'] == feature['version'] and + e['value'] == 'default' and e['variation'] == None and - e['default'] == 'c' and - e['trackEvents'] == True) + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) def test_event_for_existing_feature_with_no_user_key(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - }, - u'trackEvents': True - } + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) client = make_client(store) bad_user = { u'name': u'Bob' } - assert 'c' == client.variation('feature.key', bad_user, default='c') + assert 'default' == client.variation('feature.key', bad_user, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == bad_user and - e['value'] == 'c' and + e['version'] == feature['version'] and + e['value'] == 'default' and e['variation'] == None and - e['default'] == 'c' and - e['trackEvents'] == True) + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) def test_secure_mode_hash(): diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index be8c74c5..98bfa2bd 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -2,8 +2,10 @@ import json from ldclient.client import LDClient, Config from ldclient.feature_store import InMemoryFeatureStore +from ldclient.flag import EvaluationDetail from ldclient.versioned_data_kind import FEATURES from testing.stub_util import MockEventProcessor, MockUpdateProcessor +from testing.test_ldclient import make_off_flag_with_value user = { 'key': 'userkey' } @@ -32,6 +34,62 @@ def make_client(store): update_processor_class=MockUpdateProcessor, feature_store=store)) +def test_variation_for_existing_feature(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'value' == client.variation('feature.key', user, default='default') + +def test_variation_for_unknown_feature(): + store = InMemoryFeatureStore() + client = make_client(store) + assert 'default' == client.variation('feature.key', user, default='default') + +def test_variation_when_user_is_none(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'default' == client.variation('feature.key', None, default='default') + +def test_variation_when_user_has_no_key(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'default' == client.variation('feature.key', { }, default='default') + +def test_variation_detail_for_existing_feature(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + expected = EvaluationDetail('value', 0, {'kind': 'OFF'}) + assert expected == client.variation_detail('feature.key', user, default='default') + +def test_variation_detail_for_unknown_feature(): + store = InMemoryFeatureStore() + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'FLAG_NOT_FOUND'}) + assert expected == client.variation_detail('feature.key', user, default='default') + +def test_variation_detail_when_user_is_none(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'USER_NOT_SPECIFIED'}) + assert expected == client.variation_detail('feature.key', None, default='default') + +def test_variation_when_user_has_no_key(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'USER_NOT_SPECIFIED'}) + assert expected == client.variation_detail('feature.key', { }, default='default') + def test_all_flags_returns_values(): store = InMemoryFeatureStore() store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) @@ -79,6 +137,34 @@ def test_all_flags_state_returns_state(): '$valid': True } +def test_all_flags_state_returns_state_with_reasons(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + state = client.all_flags_state(user, with_reasons=True) + assert state.valid == True + result = state.to_json_dict() + assert result == { + 'key1': 'value1', + 'key2': 'value2', + '$flagsState': { + 'key1': { + 'variation': 0, + 'version': 100, + 'trackEvents': False, + 'reason': {'kind': 'OFF'} + }, + 'key2': { + 'variation': 1, + 'version': 200, + 'trackEvents': True, + 'debugEventsUntilDate': 1000, + 'reason': {'kind': 'OFF'} + } + }, + '$valid': True + } + def test_all_flags_state_can_be_filtered_for_client_side_flags(): flag1 = { 'key': 'server-side-1', From 0b088d4936873cabd1cd220a1a77735b5929bf5c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 23 Aug 2018 17:16:50 -0700 Subject: [PATCH 13/21] simplify default logic & add tests --- ldclient/client.py | 8 +++++--- ldclient/flag.py | 6 ++++++ testing/test_ldclient_evaluation.py | 25 +++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 32c054c0..5780ea99 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -220,9 +220,11 @@ def send_event(value, variation=None, flag=None, reason=None): result = evaluate(flag, user, self._store, include_reasons_in_events) for event in result.events or []: self._send_event(event) - value = default if result.detail.variation_index is None else result.detail.value - send_event(value, result.detail.variation_index, flag, result.detail.reason) - return EvaluationDetail(value, result.detail.variation_index, result.detail.reason) + detail = result.detail + if detail.is_default_value(): + detail = EvaluationDetail(default, None, detail.reason) + send_event(detail.value, detail.variation_index, flag, detail.reason) + return detail except Exception as e: log.error("Unexpected error while evaluating feature flag \"%s\": %s" % (key, e)) log.debug(traceback.format_exc()) diff --git a/ldclient/flag.py b/ldclient/flag.py index 3956e956..a111c2f6 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -64,6 +64,12 @@ def reason(self): """ return self.__reason + def is_default_value(self): + """Returns True if the flag evaluated to the default value rather than one of its + variations. + """ + return self.__variation_index is None + def __eq__(self, other): return self.value == other.value and self.variation_index == other.variation_index and self.reason == other.reason diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index 98bfa2bd..9183034b 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -60,6 +60,17 @@ def test_variation_when_user_has_no_key(): client = make_client(store) assert 'default' == client.variation('feature.key', { }, default='default') +def test_variation_for_flag_that_evaluates_to_none(): + empty_flag = { + 'key': 'feature.key', + 'on': False, + 'offVariation': None + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': empty_flag}}) + client = make_client(store) + assert 'default' == client.variation('feature.key', user, default='default') + def test_variation_detail_for_existing_feature(): feature = make_off_flag_with_value('feature.key', 'value') store = InMemoryFeatureStore() @@ -90,6 +101,20 @@ def test_variation_when_user_has_no_key(): expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'USER_NOT_SPECIFIED'}) assert expected == client.variation_detail('feature.key', { }, default='default') +def test_variation_detail_for_flag_that_evaluates_to_none(): + empty_flag = { + 'key': 'feature.key', + 'on': False, + 'offVariation': None + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': empty_flag}}) + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'OFF'}) + actual = client.variation_detail('feature.key', user, default='default') + assert expected == actual + assert actual.is_default_value() == True + def test_all_flags_returns_values(): store = InMemoryFeatureStore() store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) From ed69b5543c5d2ab6e54ccc8af9cbacde5120dbe6 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 14:07:48 -0700 Subject: [PATCH 14/21] add missing docstrings to client methods --- ldclient/client.py | 79 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 9 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 51167025..4e029b3d 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -28,6 +28,16 @@ class LDClient(object): def __init__(self, sdk_key=None, config=None, start_wait=5): + """Constructs a new LDClient instance. + + Rather than calling this constructor directly, you can call the `ldclient.set_sdk_key`, + `ldclient.set_config`, and `ldclient.get` functions to configure and use a singleton + client instance. + + :param string sdk_key: the SDK key for your LaunchDarkly environment + :param Config config: optional custom configuration + :param float start_wait: the number of seconds to wait for a successful connection to LaunchDarkly + """ check_uwsgi() if config is not None and config.sdk_key is not None and sdk_key is not None: @@ -94,9 +104,17 @@ def __init__(self, sdk_key=None, config=None, start_wait=5): "Feature Flags may not yet be available.") def get_sdk_key(self): + """Returns the configured SDK key. + + :rtype: string + """ return self._config.sdk_key def close(self): + """Releases all threads and network connections used by the LaunchDarkly client. + + Do not attempt to use the client after calling this method. + """ log.info("Closing LaunchDarkly client..") if self.is_offline(): return @@ -109,33 +127,66 @@ def _send_event(self, event): self._event_processor.send_event(event) def track(self, event_name, user, data=None): + """Tracks that a user performed an event. + + :param string event_name: The name of the event. + :param dict user: The attributes of the user. + :param data: Optional additional data associated with the event. + """ self._sanitize_user(user) if user is None or user.get('key') is None: log.warn("Missing user or user key when calling track().") self._send_event({'kind': 'custom', 'key': event_name, 'user': user, 'data': data}) def identify(self, user): + """Registers the user. + + :param dict user: attributes of the user to register + """ self._sanitize_user(user) if user is None or user.get('key') is None: log.warn("Missing user or user key when calling identify().") self._send_event({'kind': 'identify', 'key': user.get('key'), 'user': user}) def is_offline(self): + """Returns true if the client is in offline mode. + + :rtype: bool + """ return self._config.offline def is_initialized(self): + """Returns true if the client has successfully connected to LaunchDarkly. + + :rype: bool + """ return self.is_offline() or self._config.use_ldd or self._update_processor.initialized() def flush(self): + """Flushes all pending events. + """ if self._config.offline: return return self._event_processor.flush() def toggle(self, key, user, default): + """Deprecated synonym for `variation`. + """ log.warn("Deprecated method: toggle() called. Use variation() instead.") return self.variation(key, user, default) def variation(self, key, user, default): + """Determines the variation of a feature flag for a user. + + :param string key: the unique key for the feature flag + :param dict user: a dictionary containing parameters for the end user requesting the flag + :param object default: the default value of the flag, to be used if the value is not + available from LaunchDarkly + :return: one of the flag's variation values, or the default value + """ + return self._evaluate_internal(key, user, default).value + + def _evaluate_internal(self, key, user, default): default = self._config.get_default(key, default) if user is not None: self._sanitize_user(user) @@ -202,13 +253,13 @@ def _evaluate_and_send_events(self, flag, user, default): def all_flags(self, user): """Returns all feature flag values for the given user. - This method is deprecated - please use all_flags_state instead. Current versions of the - client-side SDK (2.0.0 and later) will not generate analytics events correctly if you pass - the result of all_flags. + This method is deprecated - please use `all_flags_state` instead. Current versions of the + client-side SDK will not generate analytics events correctly if you pass the result of `all_flags`. - :param user: the end user requesting the feature flags - :return a dictionary of feature flag keys to values; returns None if the client is offline, + :param dict user: the end user requesting the feature flags + :return: a dictionary of feature flag keys to values; returns None if the client is offline, has not been initialized, or the user is None or has no key + :rtype: dict """ state = self.all_flags_state(user) if not state.valid: @@ -217,15 +268,17 @@ def all_flags(self, user): def all_flags_state(self, user, **kwargs): """Returns an object that encapsulates the state of all feature flags for a given user, - including the flag values and also metadata that can be used on the front end. This method - does not send analytics events back to LaunchDarkly. + including the flag values and also metadata that can be used on the front end. + + This method does not send analytics events back to LaunchDarkly. - :param user: the end user requesting the feature flags + :param dict user: the end user requesting the feature flags :param kwargs: optional parameters affecting how the state is computed: set `client_side_only=True` to limit it to only flags that are marked for use with the client-side SDK (by default, all flags are included) - :return a FeatureFlagsState object (will never be None; its 'valid' property will be False + :return: a FeatureFlagsState object (will never be None; its 'valid' property will be False if the client is offline, has not been initialized, or the user is None or has no key) + :rtype: FeatureFlagsState """ if self._config.offline: log.warn("all_flags_state() called, but client is in offline mode. Returning empty state") @@ -263,6 +316,14 @@ def all_flags_state(self, user, **kwargs): return state def secure_mode_hash(self, user): + """Generates a hash value for a user. + + For more info: https://github.com/launchdarkly/js-client#secure-mode + + :param dict user: the attributes of the user + :return: a hash string that can be passed to the front end + :rtype: string + """ if user.get('key') is None or self._config.sdk_key is None: return "" return hmac.new(self._config.sdk_key.encode(), user.get('key').encode(), hashlib.sha256).hexdigest() From eb1282a937ab3bfd96af50e634bf778dc89bb67b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 14:10:04 -0700 Subject: [PATCH 15/21] revert accidental change --- ldclient/client.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 4e029b3d..99e6a085 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -184,9 +184,6 @@ def variation(self, key, user, default): available from LaunchDarkly :return: one of the flag's variation values, or the default value """ - return self._evaluate_internal(key, user, default).value - - def _evaluate_internal(self, key, user, default): default = self._config.get_default(key, default) if user is not None: self._sanitize_user(user) From 9bb5843673a4cd02daec998b3f4aedbe0f4c0625 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 14:13:22 -0700 Subject: [PATCH 16/21] typo --- ldclient/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/config.py b/ldclient/config.py index f4abd507..35af5110 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -46,7 +46,7 @@ def __init__(self, :param int events_max_pending: The capacity of the events buffer. The client buffers up to this many events in memory before flushing. If the capacity is exceeded before the buffer is flushed, events will be discarded. - : param float flush_interval: The number of seconds in between flushes of the events buffer. Decreasing + :param float flush_interval: The number of seconds in between flushes of the events buffer. Decreasing the flush interval means that the event buffer is less likely to reach capacity. :param string stream_uri: The URL for the LaunchDarkly streaming events server. Most users should use the default value. From cd82aa373b3b9212b2ef3890bb22b8e8c1744495 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 14:21:14 -0700 Subject: [PATCH 17/21] comment formatting --- ldclient/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/client.py b/ldclient/client.py index 99e6a085..52d08c55 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -274,7 +274,7 @@ def all_flags_state(self, user, **kwargs): `client_side_only=True` to limit it to only flags that are marked for use with the client-side SDK (by default, all flags are included) :return: a FeatureFlagsState object (will never be None; its 'valid' property will be False - if the client is offline, has not been initialized, or the user is None or has no key) + if the client is offline, has not been initialized, or the user is None or has no key) :rtype: FeatureFlagsState """ if self._config.offline: From 97622a35d117f6e13a2aea9427fe4ec4eb45e3f2 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 14:21:36 -0700 Subject: [PATCH 18/21] comment formatting --- ldclient/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/client.py b/ldclient/client.py index 52d08c55..d0256644 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -255,7 +255,7 @@ def all_flags(self, user): :param dict user: the end user requesting the feature flags :return: a dictionary of feature flag keys to values; returns None if the client is offline, - has not been initialized, or the user is None or has no key + has not been initialized, or the user is None or has no key :rtype: dict """ state = self.all_flags_state(user) From 3ba935265ec77009071c9dbf68538f40472bbf2c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 29 Aug 2018 11:14:54 -0700 Subject: [PATCH 19/21] fix event value when prerequisite flag is off --- ldclient/flag.py | 40 +++++++++++++++++----------------------- testing/test_flag.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/ldclient/flag.py b/ldclient/flag.py index a111c2f6..1af1a8ae 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -91,14 +91,14 @@ def error_reason(error_kind): def evaluate(flag, user, store, include_reasons_in_events = False): - if flag.get('on', False): - prereq_events = [] - detail = _evaluate(flag, user, store, prereq_events, include_reasons_in_events) - return EvalResult(detail = detail, events = prereq_events) - return EvalResult(detail = _get_off_value(flag, {'kind': 'OFF'}), events = []) - + prereq_events = [] + detail = _evaluate(flag, user, store, prereq_events, include_reasons_in_events) + return EvalResult(detail = detail, events = prereq_events) def _evaluate(flag, user, store, prereq_events, include_reasons_in_events): + if not flag.get('on', False): + return _get_off_value(flag, {'kind': 'OFF'}) + prereq_failure_reason = _check_prerequisites(flag, user, store, prereq_events, include_reasons_in_events) if prereq_failure_reason is not None: return _get_off_value(flag, prereq_failure_reason) @@ -128,25 +128,19 @@ def _check_prerequisites(flag, user, store, events, include_reasons_in_events): if prereq_flag is None: log.warn("Missing prereq flag: " + prereq.get('key')) failed_prereq = prereq - break - if prereq_flag.get('on', False) is True: + else: prereq_res = _evaluate(prereq_flag, user, store, events, include_reasons_in_events) - if prereq_res.variation_index != prereq.get('variation'): + if (not prereq_flag.get('on', False)) or prereq_res.variation_index != prereq.get('variation'): failed_prereq = prereq - else: - failed_prereq = prereq - - event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, - 'variation': prereq_res.variation_index if prereq_res else None, - 'value': prereq_res.value if prereq_res else None, - 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), - 'trackEvents': prereq_flag.get('trackEvents'), - 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate'), - 'reason': prereq_res.reason if prereq_res and include_reasons_in_events else None} - events.append(event) - - if failed_prereq: - return {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': failed_prereq.get('key')} + event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, + 'variation': prereq_res.variation_index, 'value': prereq_res.value, + 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), + 'trackEvents': prereq_flag.get('trackEvents'), + 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate'), + 'reason': prereq_res.reason if prereq_res and include_reasons_in_events else None} + events.append(event) + if failed_prereq: + return {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': failed_prereq.get('key')} return None diff --git a/testing/test_flag.py b/testing/test_flag.py index fbe54939..97f64af0 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -74,6 +74,34 @@ def test_flag_returns_off_variation_if_prerequisite_not_found(): detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'badfeature'}) assert evaluate(flag, user, empty_store) == EvalResult(detail, []) +def test_flag_returns_off_variation_and_event_if_prerequisite_is_off(): + store = InMemoryFeatureStore() + flag = { + 'key': 'feature0', + 'on': True, + 'prerequisites': [{'key': 'feature1', 'variation': 1}], + 'fallthrough': { 'variation': 0 }, + 'offVariation': 1, + 'variations': ['a', 'b', 'c'], + 'version': 1 + } + flag1 = { + 'key': 'feature1', + 'off': False, + 'offVariation': 1, + # note that even though it returns the desired variation, it is still off and therefore not a match + 'fallthrough': { 'variation': 0 }, + 'variations': ['d', 'e'], + 'version': 2, + 'trackEvents': False + } + store.upsert(FEATURES, flag1) + user = { 'key': 'x' } + detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'feature1'}) + events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None, 'reason': None}] + assert evaluate(flag, user, store) == EvalResult(detail, events_should_be) + def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): store = InMemoryFeatureStore() flag = { From 1d13ab4d2e7c86cda6524a76a43e7570a2345019 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 29 Aug 2018 11:15:44 -0700 Subject: [PATCH 20/21] comment --- ldclient/flag.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ldclient/flag.py b/ldclient/flag.py index 1af1a8ae..d4fcbdf3 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -130,6 +130,8 @@ def _check_prerequisites(flag, user, store, events, include_reasons_in_events): failed_prereq = prereq else: prereq_res = _evaluate(prereq_flag, user, store, events, include_reasons_in_events) + # Note that if the prerequisite flag is off, we don't consider it a match no matter what its + # off variation was. But we still need to evaluate it in order to generate an event. if (not prereq_flag.get('on', False)) or prereq_res.variation_index != prereq.get('variation'): failed_prereq = prereq event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, From 816bf472d8bca606260360070a848c12f67c5235 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 29 Aug 2018 17:23:18 -0700 Subject: [PATCH 21/21] version 6.4.0 --- CHANGELOG.md | 7 +++++++ ldclient/version.py | 2 +- setup.py | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f071d9b..0589a487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to the LaunchDarkly Python SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [6.4.0] - 2018-08-29 +### Added: +- The new `LDClient` method `variation_detail` allows you to evaluate a feature flag (using the same parameters as you would for `variation`) and receive more information about how the value was calculated. This information is returned in an `EvaluationDetail` object, which contains both the result value and a "reason" object which will tell you, for instance, if the user was individually targeted for the flag or was matched by one of the flag's rules, or if the flag returned the default value due to an error. + +### Fixed: +- When evaluating a prerequisite feature flag, the analytics event for the evaluation did not include the result value if the prerequisite flag was off. + ## [6.3.0] - 2018-08-27 ### Added: - The new `LDClient` method `all_flags_state()` should be used instead of `all_flags()` if you are passing flag data to the front end for use with the JavaScript SDK. It preserves some flag metadata that the front end requires in order to send analytics events correctly. Versions 2.5.0 and above of the JavaScript SDK are able to use this metadata, but the output of `all_flags_state()` will still work with older versions. diff --git a/ldclient/version.py b/ldclient/version.py index 45311203..1d01958d 100644 --- a/ldclient/version.py +++ b/ldclient/version.py @@ -1 +1 @@ -VERSION = "6.2.0" +VERSION = "6.4.0" diff --git a/setup.py b/setup.py index e471ab74..95c419f3 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ def parse_requirements(filename): return [line for line in lineiter if line and not line.startswith("#")] -ldclient_version='6.3.0' +ldclient_version='6.4.0' # parse_requirements() returns generator of pip.req.InstallRequirement objects install_reqs = parse_requirements('requirements.txt')