From 96a1cc30737e13108d9a5211a6b998db1096e8fe Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 4 Oct 2018 19:17:42 -0700 Subject: [PATCH 1/3] add option to reduce front-end metadata for untracked flags --- ldclient/client.py | 10 +++-- ldclient/flags_state.py | 12 ++++-- testing/test_flags_state.py | 21 +++++----- testing/test_ldclient_evaluation.py | 59 +++++++++++++++++++++++++++-- 4 files changed, 81 insertions(+), 21 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 683a5c3b..039fad52 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -296,7 +296,10 @@ def all_flags_state(self, user, **kwargs): :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); set `with_reasons=True` to - include evaluation reasons in the state (see `variation_detail`) + include evaluation reasons in the state (see `variation_detail`); set + `details_only_for_tracked_flags=True` to omit any metadata that is normally only + used for event generation, such as flag versions and evaluation reasons, unless + the flag has event tracking or debugging turned on :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 @@ -319,6 +322,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) + details_only_if_tracked = kwargs.get('details_only_for_tracked_flags', False) try: flags_map = self._store.all(FEATURES, lambda x: x) if flags_map is None: @@ -333,12 +337,12 @@ def all_flags_state(self, user, **kwargs): try: detail = evaluate(flag, user, self._store, False).detail state.add_flag(flag, detail.value, detail.variation_index, - detail.reason if with_reasons else None) + detail.reason if with_reasons else None, details_only_if_tracked) except Exception as e: log.error("Error evaluating flag \"%s\" in all_flags_state: %s" % (key, e)) log.debug(traceback.format_exc()) reason = {'kind': 'ERROR', 'errorKind': 'EXCEPTION'} - state.add_flag(flag, None, None, reason if with_reasons else None) + state.add_flag(flag, None, None, reason if with_reasons else None, details_only_if_tracked) return state diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py index c76b4908..cbfde1ec 100644 --- a/ldclient/flags_state.py +++ b/ldclient/flags_state.py @@ -12,15 +12,19 @@ def __init__(self, valid): self.__flag_metadata = {} self.__valid = valid - def add_flag(self, flag, value, variation, reason): + def add_flag(self, flag, value, variation, reason, details_only_if_tracked): """Used internally to build the state map.""" key = flag['key'] self.__flag_values[key] = value - meta = { 'version': flag.get('version'), 'trackEvents': flag.get('trackEvents') } + meta = {} + if (not details_only_if_tracked) or flag.get('trackEvents') or flag.get('debugEventsUntilDate'): + meta['version'] = flag.get('version') + if reason is not None: + meta['reason'] = reason if variation is not None: meta['variation'] = variation - if reason is not None: - meta['reason'] = reason + if flag.get('trackEvents'): + meta['trackEvents'] = True if flag.get('debugEventsUntilDate') is not None: meta['debugEventsUntilDate'] = flag.get('debugEventsUntilDate') self.__flag_metadata[key] = meta diff --git a/testing/test_flags_state.py b/testing/test_flags_state.py index 2fe5b123..45ea6404 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, None) + state.add_flag(flag, 'value', 1, None, False) 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, None) - state.add_flag(flag2, 'value2', 1, None) + state.add_flag(flag1, 'value1', 0, None, False) + state.add_flag(flag2, 'value2', 1, None, False) 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, None) - state.add_flag(flag2, 'value2', 1, None) + state.add_flag(flag1, 'value1', 0, None, False) + state.add_flag(flag2, 'value2', 1, None, False) result = state.to_json_dict() assert result == { @@ -35,8 +35,7 @@ def test_can_convert_to_json_dict(): '$flagsState': { 'key1': { 'variation': 0, - 'version': 100, - 'trackEvents': False + 'version': 100 }, 'key2': { 'variation': 1, @@ -52,8 +51,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, None) - state.add_flag(flag2, 'value2', 1, None) + state.add_flag(flag1, 'value1', 0, None, False) + state.add_flag(flag2, 'value2', 1, None, False) obj = state.to_json_dict() str = state.to_json_string() @@ -63,8 +62,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, None) - state.add_flag(flag2, 'value2', 1, None) + state.add_flag(flag1, 'value1', 0, None, False) + state.add_flag(flag2, 'value2', 1, None, False) obj = state.to_json_dict() str = jsonpickle.encode(state, unpicklable=False) diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index 9183034b..81719564 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -149,8 +149,7 @@ def test_all_flags_state_returns_state(): '$flagsState': { 'key1': { 'variation': 0, - 'version': 100, - 'trackEvents': False + 'version': 100 }, 'key2': { 'variation': 1, @@ -176,7 +175,6 @@ def test_all_flags_state_returns_state_with_reasons(): 'key1': { 'variation': 0, 'version': 100, - 'trackEvents': False, 'reason': {'kind': 'OFF'} }, 'key2': { @@ -229,6 +227,61 @@ def test_all_flags_state_can_be_filtered_for_client_side_flags(): values = state.to_values_map() assert values == { 'client-side-1': 'value1', 'client-side-2': 'value2' } +def test_all_flags_state_can_omit_details_for_untracked_flags(): + 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 + } + flag3 = { + 'key': 'key3', + 'version': 300, + 'on': False, + 'offVariation': 1, + 'variations': [ 'x', 'value3' ], + 'debugEventsUntilDate': 1000 + } + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2, 'key3': flag3 } }) + client = make_client(store) + state = client.all_flags_state(user, with_reasons=True, details_only_for_tracked_flags=True) + assert state.valid == True + result = state.to_json_dict() + assert result == { + 'key1': 'value1', + 'key2': 'value2', + 'key3': 'value3', + '$flagsState': { + 'key1': { + 'variation': 0 + }, + 'key2': { + 'variation': 1, + 'version': 200, + 'trackEvents': True, + 'reason': {'kind': 'OFF'} + }, + 'key3': { + 'variation': 1, + 'version': 300, + 'debugEventsUntilDate': 1000, + 'reason': {'kind': 'OFF'} + } + }, + '$valid': True + } + def test_all_flags_state_returns_empty_state_if_user_is_none(): store = InMemoryFeatureStore() store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) From 89056fc7587ba16f9573e707c82be42af14a7b20 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 8 Oct 2018 16:33:39 -0700 Subject: [PATCH 2/3] fix logic for whether a flag is tracked in all_flags_state --- ldclient/flags_state.py | 8 +++++++- testing/test_ldclient_evaluation.py | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py index cbfde1ec..c5a8ab41 100644 --- a/ldclient/flags_state.py +++ b/ldclient/flags_state.py @@ -1,4 +1,5 @@ import json +import time class FeatureFlagsState(object): """ @@ -17,7 +18,12 @@ def add_flag(self, flag, value, variation, reason, details_only_if_tracked): key = flag['key'] self.__flag_values[key] = value meta = {} - if (not details_only_if_tracked) or flag.get('trackEvents') or flag.get('debugEventsUntilDate'): + with_details = (not details_only_if_tracked) or flag.get('trackEvents') + if not with_details: + if flag.get('debugEventsUntilDate'): + now = int(time.time() * 1000) + with_details = (flag.get('debugEventsUntilDate') > now) + if with_details: meta['version'] = flag.get('version') if reason is not None: meta['reason'] = reason diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index 81719564..46c48756 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -1,5 +1,6 @@ import pytest import json +import time from ldclient.client import LDClient, Config from ldclient.feature_store import InMemoryFeatureStore from ldclient.flag import EvaluationDetail @@ -228,6 +229,7 @@ def test_all_flags_state_can_be_filtered_for_client_side_flags(): assert values == { 'client-side-1': 'value1', 'client-side-2': 'value2' } def test_all_flags_state_can_omit_details_for_untracked_flags(): + future_time = (time.time() * 1000) + 100000 flag1 = { 'key': 'key1', 'version': 100, @@ -250,7 +252,7 @@ def test_all_flags_state_can_omit_details_for_untracked_flags(): 'on': False, 'offVariation': 1, 'variations': [ 'x', 'value3' ], - 'debugEventsUntilDate': 1000 + 'debugEventsUntilDate': future_time } store = InMemoryFeatureStore() store.init({ FEATURES: { 'key1': flag1, 'key2': flag2, 'key3': flag3 } }) @@ -275,7 +277,7 @@ def test_all_flags_state_can_omit_details_for_untracked_flags(): 'key3': { 'variation': 1, 'version': 300, - 'debugEventsUntilDate': 1000, + 'debugEventsUntilDate': future_time, 'reason': {'kind': 'OFF'} } }, From 1fc23e4e5a1e4d0a1f4256df5faf1c36bf85c4eb Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sun, 14 Oct 2018 00:25:44 -0700 Subject: [PATCH 3/3] use expiringdict from PyPi --- NOTICE.txt | 2 - ldclient/expiringdict.py | 155 -------------------------------- ldclient/redis_feature_store.py | 2 +- requirements.txt | 1 + 4 files changed, 2 insertions(+), 158 deletions(-) delete mode 100644 NOTICE.txt delete mode 100644 ldclient/expiringdict.py diff --git a/NOTICE.txt b/NOTICE.txt deleted file mode 100644 index 24f9d0e4..00000000 --- a/NOTICE.txt +++ /dev/null @@ -1,2 +0,0 @@ -This product includes software (ExpiringDict) developed by -Mailgun (https://github.com/mailgun/expiringdict). \ No newline at end of file diff --git a/ldclient/expiringdict.py b/ldclient/expiringdict.py deleted file mode 100644 index 4b244c21..00000000 --- a/ldclient/expiringdict.py +++ /dev/null @@ -1,155 +0,0 @@ -''' -Dictionary with auto-expiring values for caching purposes. - -Expiration happens on any access, object is locked during cleanup from expired -values. Can not store more than max_len elements - the oldest will be deleted. - ->>> ExpiringDict(max_len=100, max_age_seconds=10) - -The values stored in the following way: -{ - key1: (value1, created_time1), - key2: (value2, created_time2) -} - -NOTE: iteration over dict and also keys() do not remove expired values! - -Copied from https://github.com/mailgun/expiringdict/commit/d17d071721dd12af6829819885a74497492d7fb7 under the APLv2 - -TODO - Use PyPI version once https://github.com/mailgun/expiringdict/issues/13 has been fixed so that -https://github.com/mailgun/expiringdict/commit/62c50ce7083a1557a1140dae19145f3a0a7a1a14 is patched -''' - -import time -from threading import RLock - -from collections import OrderedDict - - -class ExpiringDict(OrderedDict): - - def __init__(self, max_len, max_age_seconds): - assert max_age_seconds >= 0 - assert max_len >= 1 - - OrderedDict.__init__(self) - self.max_len = max_len - self.max_age = max_age_seconds - self.lock = RLock() - - def __contains__(self, key): - """ Return True if the dict has a key, else return False. """ - try: - with self.lock: - item = OrderedDict.__getitem__(self, key) - if time.time() - item[1] < self.max_age: - return True - else: - del self[key] - except KeyError: - pass - return False - - def __getitem__(self, key, with_age=False): - """ Return the item of the dict. - - Raises a KeyError if key is not in the map. - """ - with self.lock: - item = OrderedDict.__getitem__(self, key) - item_age = time.time() - item[1] - if item_age < self.max_age: - if with_age: - return item[0], item_age - else: - return item[0] - else: - del self[key] - raise KeyError(key) - - def __setitem__(self, key, value): - """ Set d[key] to value. """ - with self.lock: - if len(self) == self.max_len: - self.popitem(last=False) - OrderedDict.__setitem__(self, key, (value, time.time())) - - def pop(self, key, default=None): - """ Get item from the dict and remove it. - - Return default if expired or does not exist. Never raise KeyError. - """ - with self.lock: - try: - item = OrderedDict.__getitem__(self, key) - del self[key] - return item[0] - except KeyError: - return default - - def ttl(self, key): - """ Return TTL of the `key` (in seconds). - - Returns None for non-existent or expired keys. - """ - key_value, key_age = self.get(key, with_age=True) - if key_age: - key_ttl = self.max_age - key_age - if key_ttl > 0: - return key_ttl - return None - - def get(self, key, default=None, with_age=False): - " Return the value for key if key is in the dictionary, else default. " - try: - return self.__getitem__(key, with_age) - except KeyError: - if with_age: - return default, None - else: - return default - - def items(self): - """ Return a copy of the dictionary's list of (key, value) pairs. """ - r = [] - for key in self: - try: - r.append((key, self[key])) - except KeyError: - pass - return r - - def values(self): - """ Return a copy of the dictionary's list of values. - See the note for dict.items(). """ - r = [] - for key in self: - try: - r.append(self[key]) - except KeyError: - pass - return r - - def fromkeys(self): - " Create a new dictionary with keys from seq and values set to value. " - raise NotImplementedError() - - def iteritems(self): - """ Return an iterator over the dictionary's (key, value) pairs. """ - raise NotImplementedError() - - def itervalues(self): - """ Return an iterator over the dictionary's values. """ - raise NotImplementedError() - - def viewitems(self): - " Return a new view of the dictionary's items ((key, value) pairs). " - raise NotImplementedError() - - def viewkeys(self): - """ Return a new view of the dictionary's keys. """ - raise NotImplementedError() - - def viewvalues(self): - """ Return a new view of the dictionary's values. """ - raise NotImplementedError() diff --git a/ldclient/redis_feature_store.py b/ldclient/redis_feature_store.py index b016a1eb..71b7261b 100644 --- a/ldclient/redis_feature_store.py +++ b/ldclient/redis_feature_store.py @@ -1,10 +1,10 @@ import json from pprint import pprint +from expiringdict import ExpiringDict import redis from ldclient import log -from ldclient.expiringdict import ExpiringDict from ldclient.interfaces import FeatureStore from ldclient.memoized_value import MemoizedValue from ldclient.versioned_data_kind import FEATURES diff --git a/requirements.txt b/requirements.txt index 90a5ef51..8787ac53 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,6 @@ backoff>=1.4.3 certifi>=2018.4.16 +expiringdict>=1.1.4 future>=0.16.0 six>=1.10.0 pyRFC3339>=1.0