From 96a1cc30737e13108d9a5211a6b998db1096e8fe Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 4 Oct 2018 19:17:42 -0700 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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 From ae8b25eb33ad3dbca21231f22ece4a96694f731c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 1 Nov 2018 15:04:39 -0700 Subject: [PATCH 04/15] implement file data source, not including auto-update --- ldclient/file_data_source.py | 95 ++++++++++++++ test-requirements.txt | 3 +- testing/test_file_data_source.py | 205 +++++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 ldclient/file_data_source.py create mode 100644 testing/test_file_data_source.py diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py new file mode 100644 index 00000000..d82930d9 --- /dev/null +++ b/ldclient/file_data_source.py @@ -0,0 +1,95 @@ +import json +import six +import traceback + +have_yaml = False +try: + import yaml + have_yaml = True +except ImportError: + pass + +from ldclient.interfaces import UpdateProcessor +from ldclient.util import log +from ldclient.versioned_data_kind import FEATURES, SEGMENTS + + +class FileDataSource(UpdateProcessor): + @classmethod + def factory(cls, **kwargs): + return lambda config, store, ready : FileDataSource(store, kwargs, ready) + + def __init__(self, store, options, ready): + self._store = store + self._ready = ready + self._inited = False + self._paths = options.get('paths', []) + if isinstance(self._paths, six.string_types): + self._paths = [ self._paths ] + + def start(self): + self._load_all() + + # We will signal readiness immediately regardless of whether the file load succeeded or failed - + # the difference can be detected by checking initialized() + self._ready.set() + + def stop(self): + pass + + def initialized(self): + return self._inited + + def _load_all(self): + all_data = { FEATURES: {}, SEGMENTS: {} } + print "Loading: %s" % self._paths + for path in self._paths: + try: + self._load_file(path, all_data) + except Exception as e: + log.error('Unable to load flag data from "%s": %s' % (path, repr(e))) + traceback.print_exc() + return + print "Initing: %s" % all_data + self._store.init(all_data) + self._inited = True + + def _load_file(self, path, all_data): + content = None + with open(path, 'r') as f: + content = f.read() + parsed = self._parse_content(content) + for key, flag in six.iteritems(parsed.get('flags', {})): + self._add_item(all_data, FEATURES, flag) + for key, value in six.iteritems(parsed.get('flagValues', {})): + self._add_item(all_data, FEATURES, self._make_flag_with_value(key, value)) + for key, segment in six.iteritems(parsed.get('segments', {})): + self._add_item(all_data, SEGMENTS, segment) + + def _parse_content(self, content): + if have_yaml: + if content.strip().startswith("{"): + print("json: %s" % content) + return json.loads(content) + else: + return yaml.load(content) + print("json: %s" % content) + return json.loads(content) + + def _add_item(self, all_data, kind, item): + items = all_data[kind] + key = item.get('key') + if items.get(key) is None: + items[key] = item + else: + raise Exception('In %s, key "%s" was used more than once' % (kind.namespace, key)) + + def _make_flag_with_value(self, key, value): + return { + 'key': key, + 'on': True, + 'fallthrough': { + 'variation': 0 + }, + 'variations': [ value ] + } diff --git a/test-requirements.txt b/test-requirements.txt index ee547312..1aa5903e 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -4,4 +4,5 @@ redis>=2.10.5 coverage>=4.4 pytest-capturelog>=0.7 pytest-cov>=2.4.0 -codeclimate-test-reporter>=0.2.1 \ No newline at end of file +codeclimate-test-reporter>=0.2.1 +pyyaml>=3.0 diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py new file mode 100644 index 00000000..de4d9393 --- /dev/null +++ b/testing/test_file_data_source.py @@ -0,0 +1,205 @@ +import json +import os +import pytest +import tempfile +import threading +import time + +from ldclient.client import LDClient +from ldclient.config import Config +from ldclient.feature_store import InMemoryFeatureStore +from ldclient.file_data_source import FileDataSource +from ldclient.versioned_data_kind import FEATURES, SEGMENTS + + +all_flag_keys = [ 'flag1', 'flag2' ] +all_properties_json = ''' + { + "flags": { + "flag1": { + "key": "flag1", + "on": true, + "fallthrough": { + "variation": 2 + }, + "variations": [ "fall", "off", "on" ] + } + }, + "flagValues": { + "flag2": "value2" + }, + "segments": { + "seg1": { + "key": "seg1", + "include": ["user1"] + } + } + } +''' + +all_properties_yaml = ''' +--- +flags: + flag1: + key: flag1 + "on": true +flagValues: + flag2: value2 +segments: + seg1: + key: seg1 + include: ["user1"] +''' + +flag_only_json = ''' + { + "flags": { + "flag1": { + "key": "flag1", + "on": true, + "fallthrough": { + "variation": 2 + }, + "variations": [ "fall", "off", "on" ] + } + } + } +''' + +segment_only_json = ''' + { + "segments": { + "seg1": { + "key": "seg1", + "include": ["user1"] + } + } + } +''' + +fds = None +store = None +ready = None + + +def setup_function(): + global fds, store, ready + store = InMemoryFeatureStore() + ready = threading.Event() + +def teardown_function(): + if fds is not None: + fds.stop() + +def make_temp_file(content): + f, path = tempfile.mkstemp() + os.write(f, content) + os.close(f) + return path + +def replace_file(path, content): + with open(path, 'w') as f: + f.write(content) + +def test_does_not_load_data_prior_to_start(): + path = make_temp_file('{"flagValues":{"key":"value"}}') + try: + fds = FileDataSource.factory(paths = path)(Config(), store, ready) + assert ready.is_set() is False + assert fds.initialized() is False + assert store.initialized is False + finally: + os.remove(path) + +def test_loads_flags_on_start_from_json(): + path = make_temp_file(all_properties_json) + try: + fds = FileDataSource.factory(paths = path)(Config(), store, ready) + fds.start() + assert store.initialized is True + assert sorted(list(store.all(FEATURES, lambda x: x).keys())) == all_flag_keys + finally: + os.remove(path) + +def test_loads_flags_on_start_from_yaml(): + path = make_temp_file(all_properties_yaml) + try: + fds = FileDataSource.factory(paths = path)(Config(), store, ready) + fds.start() + assert store.initialized is True + assert sorted(list(store.all(FEATURES, lambda x: x).keys())) == all_flag_keys + finally: + os.remove(path) + +def test_sets_ready_event_and_initialized_on_successful_load(): + path = make_temp_file(all_properties_json) + try: + fds = FileDataSource.factory(paths = path)(Config(), store, ready) + fds.start() + assert fds.initialized() is True + assert ready.is_set() is True + finally: + os.remove(path) + +def test_sets_ready_event_and_does_not_set_initialized_on_unsuccessful_load(): + bad_file_path = 'no-such-file' + fds = FileDataSource.factory(paths = bad_file_path)(Config(), store, ready) + fds.start() + assert fds.initialized() is False + assert ready.is_set() is True + +def test_can_load_multiple_files(): + path1 = make_temp_file(flag_only_json) + path2 = make_temp_file(segment_only_json) + try: + fds = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) + fds.start() + assert len(store.all(FEATURES, lambda x: x)) == 1 + assert len(store.all(SEGMENTS, lambda x: x)) == 1 + finally: + os.remove(path1) + os.remove(path2) + +def test_does_not_allow_duplicate_keys(): + path1 = make_temp_file(flag_only_json) + path2 = make_temp_file(flag_only_json) + try: + fds = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) + fds.start() + assert len(store.all(FEATURES, lambda x: x)) == 0 + finally: + os.remove(path1) + os.remove(path2) + +def test_does_not_reload_modified_file_if_auto_update_is_off(): + path = make_temp_file(flag_only_json) + try: + fds = FileDataSource.factory(paths = path)(Config(), store, ready) + fds.start() + assert len(store.all(SEGMENTS, lambda x: x)) == 0 + time.sleep(0.5) + replace_file(path, segment_only_json) + time.sleep(0.5) + assert len(store.all(SEGMENTS, lambda x: x)) == 0 + finally: + os.remove(path) + +def test_evaluates_full_flag_with_client_as_expected(): + path = make_temp_file(all_properties_json) + try: + fds = FileDataSource.factory(paths = path) + client = LDClient(config=Config(update_processor_class = fds, send_events = False)) + value = client.variation('flag1', { 'key': 'user' }, '') + assert value == 'on' + finally: + os.remove(path) + +def test_evaluates_simplified_flag_with_client_as_expected(): + path = make_temp_file(all_properties_json) + try: + fds = FileDataSource.factory(paths = path) + client = LDClient(config=Config(update_processor_class = fds, send_events = False)) + value = client.variation('flag2', { 'key': 'user' }, '') + assert value == 'value2' + finally: + os.remove(path) From 850837d72794b6e5e175590304fe844b808213d8 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 1 Nov 2018 15:53:56 -0700 Subject: [PATCH 05/15] rm debugging --- ldclient/file_data_source.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index d82930d9..5ebb062d 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -42,7 +42,6 @@ def initialized(self): def _load_all(self): all_data = { FEATURES: {}, SEGMENTS: {} } - print "Loading: %s" % self._paths for path in self._paths: try: self._load_file(path, all_data) From aa7684a5181143a0d7c0874c3d01f1b837f4c3b2 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 1 Nov 2018 16:01:26 -0700 Subject: [PATCH 06/15] rm debugging --- ldclient/file_data_source.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index 5ebb062d..a8351ba6 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -49,7 +49,6 @@ def _load_all(self): log.error('Unable to load flag data from "%s": %s' % (path, repr(e))) traceback.print_exc() return - print "Initing: %s" % all_data self._store.init(all_data) self._inited = True @@ -68,11 +67,9 @@ def _load_file(self, path, all_data): def _parse_content(self, content): if have_yaml: if content.strip().startswith("{"): - print("json: %s" % content) return json.loads(content) else: return yaml.load(content) - print("json: %s" % content) return json.loads(content) def _add_item(self, all_data, kind, item): From 39c90424302e934502db4cff01e9f9de96cd2e65 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 1 Nov 2018 16:30:34 -0700 Subject: [PATCH 07/15] Python 3 compatibility fix --- testing/test_file_data_source.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index de4d9393..9b4a2c7b 100644 --- a/testing/test_file_data_source.py +++ b/testing/test_file_data_source.py @@ -1,6 +1,7 @@ import json import os import pytest +import six import tempfile import threading import time @@ -93,7 +94,7 @@ def teardown_function(): def make_temp_file(content): f, path = tempfile.mkstemp() - os.write(f, content) + os.write(f, six.b(content)) os.close(f) return path From a43bf0c56789f26f80199e260ca04c4b9cb6b918 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:14:55 -0700 Subject: [PATCH 08/15] add file watching, update documentation and tests --- ldclient/file_data_source.py | 194 ++++++++++++++++++++++++++++++- test-requirements.txt | 1 + testing/test_file_data_source.py | 76 +++++++----- 3 files changed, 239 insertions(+), 32 deletions(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index a8351ba6..09afa263 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -1,4 +1,5 @@ import json +import os import six import traceback @@ -9,7 +10,17 @@ except ImportError: pass +have_watchdog = False +try: + import watchdog + import watchdog.events + import watchdog.observers + have_watchdog = True +except ImportError: + pass + from ldclient.interfaces import UpdateProcessor +from ldclient.repeating_timer import RepeatingTimer from ldclient.util import log from ldclient.versioned_data_kind import FEATURES, SEGMENTS @@ -17,6 +28,101 @@ class FileDataSource(UpdateProcessor): @classmethod def factory(cls, **kwargs): + """Provides a way to use local files as a source of feature flag state. This would typically be + used in a test environment, to operate using a predetermined feature flag state without an + actual LaunchDarkly connection. + + To use this component, call `FileDataSource.factory`, and store its return value in the + `update_processor_class` property of your LaunchDarkly client configuration. In the options + to `factory`, set `paths` to the file path(s) of your data file(s): + :: + + factory = FileDataSource.factory(paths: [ myFilePath ]) + config = Config(update_processor_class = factory) + + This will cause the client not to connect to LaunchDarkly to get feature flags. The + client may still make network connections to send analytics events, unless you have disabled + this with Config.send_events or Config.offline. + + Flag data files can be either JSON or YAML (in order to use YAML, you must install the 'pyyaml' + package). They contain an object with three possible properties: + + * "flags": Feature flag definitions. + * "flagValues": Simplified feature flags that contain only a value. + * "segments": User segment definitions. + + The format of the data in "flags" and "segments" is defined by the LaunchDarkly application + and is subject to change. Rather than trying to construct these objects yourself, it is simpler + to request existing flags directly from the LaunchDarkly server in JSON format, and use this + output as the starting point for your file. In Linux you would do this: + :: + + curl -H "Authorization: {your sdk key}" https://app.launchdarkly.com/sdk/latest-all + + The output will look something like this (but with many more properties): + :: + + { + "flags": { + "flag-key-1": { + "key": "flag-key-1", + "on": true, + "variations": [ "a", "b" ] + } + }, + "segments": { + "segment-key-1": { + "key": "segment-key-1", + "includes": [ "user-key-1" ] + } + } + } + + Data in this format allows the SDK to exactly duplicate all the kinds of flag behavior supported + by LaunchDarkly. However, in many cases you will not need this complexity, but will just want to + set specific flag keys to specific values. For that, you can use a much simpler format: + :: + + { + "flagValues": { + "my-string-flag-key": "value-1", + "my-boolean-flag-key": true, + "my-integer-flag-key": 3 + } + } + + Or, in YAML: + :: + + flagValues: + my-string-flag-key: "value-1" + my-boolean-flag-key: true + my-integer-flag-key: 1 + + It is also possible to specify both "flags" and "flagValues", if you want some flags + to have simple values and others to have complex behavior. However, it is an error to use the + same flag key or segment key more than once, either in a single file or across multiple files. + + If the data source encounters any error in any file-- malformed content, a missing file, or a + duplicate key-- it will not load flags from any of the files. + + :param kwargs: + See below + + :Keyword arguments: + * **paths** (array): The paths of the source files for loading flag data. These may be absolute paths + or relative to the current working directory. Files will be parsed as JSON unless the 'pyyaml' + package is installed, in which case YAML is also allowed. + * **auto_update** (boolean): True if the data source should watch for changes to the source file(s) + and reload flags whenever there is a change. The default implementation of this feature is based on + polling the filesystem, which may not perform well; if you install the 'watchdog' package (not + included by default, to avoid adding unwanted dependencies to the SDK), its native file watching + mechanism will be used instead. Note that auto-updating will only work if all of the files you + specified have valid directory paths at startup time. + * **poll_interval** (float): The minimum interval, in seconds, between checks for file modifications - + used only if auto_update is true, and if the native file-watching mechanism from 'watchdog' is not + being used. The default value is 1 second. + """ return lambda config, store, ready : FileDataSource(store, kwargs, ready) def __init__(self, store, options, ready): @@ -26,16 +132,25 @@ def __init__(self, store, options, ready): self._paths = options.get('paths', []) if isinstance(self._paths, six.string_types): self._paths = [ self._paths ] - + self._auto_update = options.get('auto_update', False) + self._poll_interval = options.get('poll_interval', 1) + self._force_polling = options.get('force_polling', False) # used only in tests + def start(self): self._load_all() + if self._auto_update: + self._auto_updater = self._start_auto_updater() + else: + self._auto_updater = None + # We will signal readiness immediately regardless of whether the file load succeeded or failed - # the difference can be detected by checking initialized() self._ready.set() def stop(self): - pass + if self._auto_updater: + self._auto_updater.stop() def initialized(self): return self._inited @@ -66,10 +181,7 @@ def _load_file(self, path, all_data): def _parse_content(self, content): if have_yaml: - if content.strip().startswith("{"): - return json.loads(content) - else: - return yaml.load(content) + return yaml.load(content) # pyyaml correctly parses JSON too return json.loads(content) def _add_item(self, all_data, kind, item): @@ -89,3 +201,73 @@ def _make_flag_with_value(self, key, value): }, 'variations': [ value ] } + + def _start_auto_updater(self): + resolved_paths = [] + for path in self._paths: + try: + resolved_paths.append(os.path.realpath(path)) + except: + log.warn('Cannot watch for changes to data file "%s" because it is an invalid path' % path) + if have_watchdog and not self._force_polling: + return FileDataSource.WatchdogAutoUpdater(resolved_paths, self._load_all) + else: + return FileDataSource.PollingAutoUpdater(resolved_paths, self._load_all, self._poll_interval) + + # Watch for changes to data files using the watchdog package. This uses native OS filesystem notifications + # if available for the current platform. + class WatchdogAutoUpdater(object): + def __init__(self, resolved_paths, reloader): + watched_files = set(resolved_paths) + + class LDWatchdogHandler(watchdog.events.FileSystemEventHandler): + def on_any_event(self, event): + if event.src_path in watched_files: + reloader() + + dir_paths = set() + for path in resolved_paths: + dir_paths.add(os.path.dirname(path)) + + self._observer = watchdog.observers.Observer() + handler = LDWatchdogHandler() + for path in dir_paths: + self._observer.schedule(handler, path) + self._observer.start() + + def stop(self): + self._observer.stop() + self._observer.join() + + # Watch for changes to data files by polling their modification times. This is used if auto-update is + # on but the watchdog package is not installed. + class PollingAutoUpdater(object): + def __init__(self, resolved_paths, reloader, interval): + self._paths = resolved_paths + self._reloader = reloader + self._file_times = self._check_file_times() + self._timer = RepeatingTimer(interval, self._poll) + self._timer.start() + + def stop(self): + self._timer.stop() + + def _poll(self): + new_times = self._check_file_times() + changed = False + for file_path, file_time in six.iteritems(self._file_times): + if new_times.get(file_path) is not None and new_times.get(file_path) != file_time: + changed = True + break + self._file_times = new_times + if changed: + self._reloader() + + def _check_file_times(self): + ret = {} + for path in self._paths: + try: + ret[path] = os.path.getmtime(path) + except: + ret[path] = None + return ret diff --git a/test-requirements.txt b/test-requirements.txt index 1aa5903e..413ef355 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -6,3 +6,4 @@ pytest-capturelog>=0.7 pytest-cov>=2.4.0 codeclimate-test-reporter>=0.2.1 pyyaml>=3.0 +watchdog>=0.9 diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index 9b4a2c7b..4fa16bff 100644 --- a/testing/test_file_data_source.py +++ b/testing/test_file_data_source.py @@ -78,19 +78,19 @@ } ''' -fds = None +data_source = None store = None ready = None def setup_function(): - global fds, store, ready + global data_source, store, ready store = InMemoryFeatureStore() ready = threading.Event() def teardown_function(): - if fds is not None: - fds.stop() + if data_source is not None: + data_source.stop() def make_temp_file(content): f, path = tempfile.mkstemp() @@ -105,9 +105,9 @@ def replace_file(path, content): def test_does_not_load_data_prior_to_start(): path = make_temp_file('{"flagValues":{"key":"value"}}') try: - fds = FileDataSource.factory(paths = path)(Config(), store, ready) + data_source = FileDataSource.factory(paths = path)(Config(), store, ready) assert ready.is_set() is False - assert fds.initialized() is False + assert data_source.initialized() is False assert store.initialized is False finally: os.remove(path) @@ -115,8 +115,8 @@ def test_does_not_load_data_prior_to_start(): def test_loads_flags_on_start_from_json(): path = make_temp_file(all_properties_json) try: - fds = FileDataSource.factory(paths = path)(Config(), store, ready) - fds.start() + data_source = FileDataSource.factory(paths = path)(Config(), store, ready) + data_source.start() assert store.initialized is True assert sorted(list(store.all(FEATURES, lambda x: x).keys())) == all_flag_keys finally: @@ -125,8 +125,8 @@ def test_loads_flags_on_start_from_json(): def test_loads_flags_on_start_from_yaml(): path = make_temp_file(all_properties_yaml) try: - fds = FileDataSource.factory(paths = path)(Config(), store, ready) - fds.start() + data_source = FileDataSource.factory(paths = path)(Config(), store, ready) + data_source.start() assert store.initialized is True assert sorted(list(store.all(FEATURES, lambda x: x).keys())) == all_flag_keys finally: @@ -135,26 +135,26 @@ def test_loads_flags_on_start_from_yaml(): def test_sets_ready_event_and_initialized_on_successful_load(): path = make_temp_file(all_properties_json) try: - fds = FileDataSource.factory(paths = path)(Config(), store, ready) - fds.start() - assert fds.initialized() is True + data_source = FileDataSource.factory(paths = path)(Config(), store, ready) + data_source.start() + assert data_source.initialized() is True assert ready.is_set() is True finally: os.remove(path) def test_sets_ready_event_and_does_not_set_initialized_on_unsuccessful_load(): bad_file_path = 'no-such-file' - fds = FileDataSource.factory(paths = bad_file_path)(Config(), store, ready) - fds.start() - assert fds.initialized() is False + data_source = FileDataSource.factory(paths = bad_file_path)(Config(), store, ready) + data_source.start() + assert data_source.initialized() is False assert ready.is_set() is True def test_can_load_multiple_files(): path1 = make_temp_file(flag_only_json) path2 = make_temp_file(segment_only_json) try: - fds = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) - fds.start() + data_source = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) + data_source.start() assert len(store.all(FEATURES, lambda x: x)) == 1 assert len(store.all(SEGMENTS, lambda x: x)) == 1 finally: @@ -165,8 +165,8 @@ def test_does_not_allow_duplicate_keys(): path1 = make_temp_file(flag_only_json) path2 = make_temp_file(flag_only_json) try: - fds = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) - fds.start() + data_source = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) + data_source.start() assert len(store.all(FEATURES, lambda x: x)) == 0 finally: os.remove(path1) @@ -175,8 +175,8 @@ def test_does_not_allow_duplicate_keys(): def test_does_not_reload_modified_file_if_auto_update_is_off(): path = make_temp_file(flag_only_json) try: - fds = FileDataSource.factory(paths = path)(Config(), store, ready) - fds.start() + data_source = FileDataSource.factory(paths = path)(Config(), store, ready) + data_source.start() assert len(store.all(SEGMENTS, lambda x: x)) == 0 time.sleep(0.5) replace_file(path, segment_only_json) @@ -185,22 +185,46 @@ def test_does_not_reload_modified_file_if_auto_update_is_off(): finally: os.remove(path) +def do_auto_update_test(options): + path = make_temp_file(flag_only_json) + options['paths'] = path + try: + data_source = FileDataSource.factory(**options)(Config(), store, ready) + data_source.start() + assert len(store.all(SEGMENTS, lambda x: x)) == 0 + time.sleep(0.5) + replace_file(path, segment_only_json) + time.sleep(0.5) + assert len(store.all(SEGMENTS, lambda x: x)) == 1 + finally: + os.remove(path) + +def test_reloads_modified_file_if_auto_update_is_on(): + do_auto_update_test({ 'auto_update': True }) + +def test_reloads_modified_file_in_polling_mode(): + do_auto_update_test({ 'auto_update': True, 'force_polling': True, 'poll_interval': 0.1 }) + def test_evaluates_full_flag_with_client_as_expected(): path = make_temp_file(all_properties_json) try: - fds = FileDataSource.factory(paths = path) - client = LDClient(config=Config(update_processor_class = fds, send_events = False)) + data_source = FileDataSource.factory(paths = path) + client = LDClient(config=Config(update_processor_class = data_source, send_events = False)) value = client.variation('flag1', { 'key': 'user' }, '') assert value == 'on' finally: os.remove(path) + if client is not None: + client.close() def test_evaluates_simplified_flag_with_client_as_expected(): path = make_temp_file(all_properties_json) try: - fds = FileDataSource.factory(paths = path) - client = LDClient(config=Config(update_processor_class = fds, send_events = False)) + data_source = FileDataSource.factory(paths = path) + client = LDClient(config=Config(update_processor_class = data_source, send_events = False)) value = client.variation('flag2', { 'key': 'user' }, '') assert value == 'value2' finally: os.remove(path) + if client is not None: + client.close() From 2cea73061eaba3d4d7ac812e9fbf9fffb7de5712 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:19:29 -0700 Subject: [PATCH 09/15] readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 57aac968..edef13e6 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,10 @@ Supported Python versions ---------- The SDK is tested with the most recent patch releases of Python 2.7, 3.3, 3.4, 3.5, and 3.6. Python 2.6 is no longer supported. +Using flag data from a file +--------------------------- +For testing purposes, the SDK can be made to read feature flag state from a file or files instead of connecting to LaunchDarkly. See [`file_data_source.py`](https://github.com/launchdarkly/python-client/blob/master/ldclient/file_data_source.py) for more details. + Learn more ----------- From dcf1afe6f7f1fd1535450a36fd26af18afd5c6af Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:20:42 -0700 Subject: [PATCH 10/15] debugging --- ldclient/file_data_source.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index 09afa263..79d9655f 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -218,10 +218,12 @@ def _start_auto_updater(self): # if available for the current platform. class WatchdogAutoUpdater(object): def __init__(self, resolved_paths, reloader): + print("*** all paths: %s" % resolved_paths) watched_files = set(resolved_paths) class LDWatchdogHandler(watchdog.events.FileSystemEventHandler): def on_any_event(self, event): + print("*** got event: %s" % event.src_path) if event.src_path in watched_files: reloader() @@ -232,6 +234,7 @@ def on_any_event(self, event): self._observer = watchdog.observers.Observer() handler = LDWatchdogHandler() for path in dir_paths: + print("*** watching: %s" % path) self._observer.schedule(handler, path) self._observer.start() From 4e98fdd3f3c0e0ecfbf608643ad17268c92925fa Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:35:03 -0700 Subject: [PATCH 11/15] debugging --- testing/test_file_data_source.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index 4fa16bff..7e565c17 100644 --- a/testing/test_file_data_source.py +++ b/testing/test_file_data_source.py @@ -194,8 +194,14 @@ def do_auto_update_test(options): assert len(store.all(SEGMENTS, lambda x: x)) == 0 time.sleep(0.5) replace_file(path, segment_only_json) - time.sleep(0.5) - assert len(store.all(SEGMENTS, lambda x: x)) == 1 + print("*** modified file %s" % path) + deadline = time.time() + 10 + while time.time() < deadline: + time.sleep(0.1) + if len(store.all(SEGMENTS, lambda x: x)) == 1: + return + print("*** checked") + assert False, "Flags were not reloaded after 10 seconds" finally: os.remove(path) From 8f3c2217805da177d412a6a5543982ad3e118ca6 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:41:52 -0700 Subject: [PATCH 12/15] debugging --- ldclient/file_data_source.py | 2 ++ testing/test_file_data_source.py | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index 79d9655f..c1be6974 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -239,6 +239,7 @@ def on_any_event(self, event): self._observer.start() def stop(self): + print("*** stopping observer") self._observer.stop() self._observer.join() @@ -253,6 +254,7 @@ def __init__(self, resolved_paths, reloader, interval): self._timer.start() def stop(self): + print("*** stopping polling") self._timer.stop() def _poll(self): diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index 7e565c17..e62fff62 100644 --- a/testing/test_file_data_source.py +++ b/testing/test_file_data_source.py @@ -199,16 +199,19 @@ def do_auto_update_test(options): while time.time() < deadline: time.sleep(0.1) if len(store.all(SEGMENTS, lambda x: x)) == 1: + print("*** success on %s" % path) return - print("*** checked") + print("*** checked %s" % path) assert False, "Flags were not reloaded after 10 seconds" finally: os.remove(path) def test_reloads_modified_file_if_auto_update_is_on(): + print("*** with watchdog") do_auto_update_test({ 'auto_update': True }) def test_reloads_modified_file_in_polling_mode(): + print("*** with polling") do_auto_update_test({ 'auto_update': True, 'force_polling': True, 'poll_interval': 0.1 }) def test_evaluates_full_flag_with_client_as_expected(): From 84276ddc908a1de2fae9922aaaf538d5eac560a1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:48:26 -0700 Subject: [PATCH 13/15] fix cleanup logic --- ldclient/file_data_source.py | 3 +- testing/test_file_data_source.py | 53 +++++++++++++++++--------------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index c1be6974..0b51cfdd 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -133,6 +133,7 @@ def __init__(self, store, options, ready): if isinstance(self._paths, six.string_types): self._paths = [ self._paths ] self._auto_update = options.get('auto_update', False) + self._auto_updater = None self._poll_interval = options.get('poll_interval', 1) self._force_polling = options.get('force_polling', False) # used only in tests @@ -141,8 +142,6 @@ def start(self): if self._auto_update: self._auto_updater = self._start_auto_updater() - else: - self._auto_updater = None # We will signal readiness immediately regardless of whether the file load succeeded or failed - # the difference can be detected by checking initialized() diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index e62fff62..8a8f5d5a 100644 --- a/testing/test_file_data_source.py +++ b/testing/test_file_data_source.py @@ -92,6 +92,11 @@ def teardown_function(): if data_source is not None: data_source.stop() +def make_data_source(**kwargs): + global data_source + data_source = FileDataSource.factory(**kwargs)(Config(), store, ready) + return data_source + def make_temp_file(content): f, path = tempfile.mkstemp() os.write(f, six.b(content)) @@ -105,9 +110,9 @@ def replace_file(path, content): def test_does_not_load_data_prior_to_start(): path = make_temp_file('{"flagValues":{"key":"value"}}') try: - data_source = FileDataSource.factory(paths = path)(Config(), store, ready) + source = make_data_source(paths = path) assert ready.is_set() is False - assert data_source.initialized() is False + assert source.initialized() is False assert store.initialized is False finally: os.remove(path) @@ -115,8 +120,8 @@ def test_does_not_load_data_prior_to_start(): def test_loads_flags_on_start_from_json(): path = make_temp_file(all_properties_json) try: - data_source = FileDataSource.factory(paths = path)(Config(), store, ready) - data_source.start() + source = make_data_source(paths = path) + source.start() assert store.initialized is True assert sorted(list(store.all(FEATURES, lambda x: x).keys())) == all_flag_keys finally: @@ -125,8 +130,8 @@ def test_loads_flags_on_start_from_json(): def test_loads_flags_on_start_from_yaml(): path = make_temp_file(all_properties_yaml) try: - data_source = FileDataSource.factory(paths = path)(Config(), store, ready) - data_source.start() + source = make_data_source(paths = path) + source.start() assert store.initialized is True assert sorted(list(store.all(FEATURES, lambda x: x).keys())) == all_flag_keys finally: @@ -135,26 +140,26 @@ def test_loads_flags_on_start_from_yaml(): def test_sets_ready_event_and_initialized_on_successful_load(): path = make_temp_file(all_properties_json) try: - data_source = FileDataSource.factory(paths = path)(Config(), store, ready) - data_source.start() - assert data_source.initialized() is True + source = make_data_source(paths = path) + source.start() + assert source.initialized() is True assert ready.is_set() is True finally: os.remove(path) def test_sets_ready_event_and_does_not_set_initialized_on_unsuccessful_load(): bad_file_path = 'no-such-file' - data_source = FileDataSource.factory(paths = bad_file_path)(Config(), store, ready) - data_source.start() - assert data_source.initialized() is False + source = make_data_source(paths = bad_file_path) + source.start() + assert source.initialized() is False assert ready.is_set() is True def test_can_load_multiple_files(): path1 = make_temp_file(flag_only_json) path2 = make_temp_file(segment_only_json) try: - data_source = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) - data_source.start() + source = make_data_source(paths = [ path1, path2 ]) + source.start() assert len(store.all(FEATURES, lambda x: x)) == 1 assert len(store.all(SEGMENTS, lambda x: x)) == 1 finally: @@ -165,8 +170,8 @@ def test_does_not_allow_duplicate_keys(): path1 = make_temp_file(flag_only_json) path2 = make_temp_file(flag_only_json) try: - data_source = FileDataSource.factory(paths = [ path1, path2 ])(Config(), store, ready) - data_source.start() + source = make_data_source(paths = [ path1, path2 ]) + source.start() assert len(store.all(FEATURES, lambda x: x)) == 0 finally: os.remove(path1) @@ -175,8 +180,8 @@ def test_does_not_allow_duplicate_keys(): def test_does_not_reload_modified_file_if_auto_update_is_off(): path = make_temp_file(flag_only_json) try: - data_source = FileDataSource.factory(paths = path)(Config(), store, ready) - data_source.start() + source = make_data_source(paths = path) + source.start() assert len(store.all(SEGMENTS, lambda x: x)) == 0 time.sleep(0.5) replace_file(path, segment_only_json) @@ -189,8 +194,8 @@ def do_auto_update_test(options): path = make_temp_file(flag_only_json) options['paths'] = path try: - data_source = FileDataSource.factory(**options)(Config(), store, ready) - data_source.start() + source = make_data_source(**options) + source.start() assert len(store.all(SEGMENTS, lambda x: x)) == 0 time.sleep(0.5) replace_file(path, segment_only_json) @@ -217,8 +222,8 @@ def test_reloads_modified_file_in_polling_mode(): def test_evaluates_full_flag_with_client_as_expected(): path = make_temp_file(all_properties_json) try: - data_source = FileDataSource.factory(paths = path) - client = LDClient(config=Config(update_processor_class = data_source, send_events = False)) + factory = FileDataSource.factory(paths = path) + client = LDClient(config=Config(update_processor_class = factory, send_events = False)) value = client.variation('flag1', { 'key': 'user' }, '') assert value == 'on' finally: @@ -229,8 +234,8 @@ def test_evaluates_full_flag_with_client_as_expected(): def test_evaluates_simplified_flag_with_client_as_expected(): path = make_temp_file(all_properties_json) try: - data_source = FileDataSource.factory(paths = path) - client = LDClient(config=Config(update_processor_class = data_source, send_events = False)) + factory = FileDataSource.factory(paths = path) + client = LDClient(config=Config(update_processor_class = factory, send_events = False)) value = client.variation('flag2', { 'key': 'user' }, '') assert value == 'value2' finally: From 2a822e6e82a1e8dffcdfd59d183d43219dff391c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 2 Nov 2018 13:50:31 -0700 Subject: [PATCH 14/15] rm debugging --- ldclient/file_data_source.py | 5 ----- testing/test_file_data_source.py | 5 ----- 2 files changed, 10 deletions(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index 0b51cfdd..c4013a52 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -217,12 +217,10 @@ def _start_auto_updater(self): # if available for the current platform. class WatchdogAutoUpdater(object): def __init__(self, resolved_paths, reloader): - print("*** all paths: %s" % resolved_paths) watched_files = set(resolved_paths) class LDWatchdogHandler(watchdog.events.FileSystemEventHandler): def on_any_event(self, event): - print("*** got event: %s" % event.src_path) if event.src_path in watched_files: reloader() @@ -233,12 +231,10 @@ def on_any_event(self, event): self._observer = watchdog.observers.Observer() handler = LDWatchdogHandler() for path in dir_paths: - print("*** watching: %s" % path) self._observer.schedule(handler, path) self._observer.start() def stop(self): - print("*** stopping observer") self._observer.stop() self._observer.join() @@ -253,7 +249,6 @@ def __init__(self, resolved_paths, reloader, interval): self._timer.start() def stop(self): - print("*** stopping polling") self._timer.stop() def _poll(self): diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index 8a8f5d5a..68d1e5b7 100644 --- a/testing/test_file_data_source.py +++ b/testing/test_file_data_source.py @@ -199,24 +199,19 @@ def do_auto_update_test(options): assert len(store.all(SEGMENTS, lambda x: x)) == 0 time.sleep(0.5) replace_file(path, segment_only_json) - print("*** modified file %s" % path) deadline = time.time() + 10 while time.time() < deadline: time.sleep(0.1) if len(store.all(SEGMENTS, lambda x: x)) == 1: - print("*** success on %s" % path) return - print("*** checked %s" % path) assert False, "Flags were not reloaded after 10 seconds" finally: os.remove(path) def test_reloads_modified_file_if_auto_update_is_on(): - print("*** with watchdog") do_auto_update_test({ 'auto_update': True }) def test_reloads_modified_file_in_polling_mode(): - print("*** with polling") do_auto_update_test({ 'auto_update': True, 'force_polling': True, 'poll_interval': 0.1 }) def test_evaluates_full_flag_with_client_as_expected(): From ac5e8de65036434ee0be93dee64c7179a9200b50 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 13 Nov 2018 20:39:44 -0800 Subject: [PATCH 15/15] typo in comment --- ldclient/file_data_source.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/file_data_source.py b/ldclient/file_data_source.py index c4013a52..ebff765b 100644 --- a/ldclient/file_data_source.py +++ b/ldclient/file_data_source.py @@ -37,7 +37,7 @@ def factory(cls, **kwargs): to `factory`, set `paths` to the file path(s) of your data file(s): :: - factory = FileDataSource.factory(paths: [ myFilePath ]) + factory = FileDataSource.factory(paths = [ myFilePath ]) config = Config(update_processor_class = factory) This will cause the client not to connect to LaunchDarkly to get feature flags. The