From 89671a634c0589683c7a55ecfce2b83d92691caf Mon Sep 17 00:00:00 2001 From: Bilal Al-Shahwany Date: Mon, 28 Aug 2023 12:32:03 -0700 Subject: [PATCH 1/5] Added "update" to split storage to replace put, remove and set_change_number --- splitio/storage/__init__.py | 35 ++++-------------- splitio/storage/inmemmory.py | 23 ++++++++++-- splitio/storage/pluggable.py | 31 +++++++++++----- splitio/storage/redis.py | 33 ++++------------- tests/storage/test_inmemory_storage.py | 51 +++++++++++--------------- 5 files changed, 79 insertions(+), 94 deletions(-) diff --git a/splitio/storage/__init__.py b/splitio/storage/__init__.py index 5467bc14..a701ac8e 100644 --- a/splitio/storage/__init__.py +++ b/splitio/storage/__init__.py @@ -30,25 +30,16 @@ def fetch_many(self, split_names): pass @abc.abstractmethod - def put(self, split): + def update(self, to_add, to_delete, new_change_number): """ - Store a split. - - :param split: Split object to store - :type split_name: splitio.models.splits.Split - """ - pass + Update feature flag strage. - @abc.abstractmethod - def remove(self, split_name): - """ - Remove a split from storage. - - :param split_name: Name of the feature to remove. - :type split_name: str - - :return: True if the split was found and removed. False otherwise. - :rtype: bool + :param to_add: List of feature flags to add + :type to_add: list[splitio.models.splits.Split] + :param to_delete: List of feature flags to delete + :type to_delete: list[splitio.models.splits.Split] + :param new_change_number: New change number. + :type new_change_number: int """ pass @@ -61,16 +52,6 @@ def get_change_number(self): """ pass - @abc.abstractmethod - def set_change_number(self, new_change_number): - """ - Set the latest change number. - - :param new_change_number: New change number. - :type new_change_number: int - """ - pass - @abc.abstractmethod def get_split_names(self): """ diff --git a/splitio/storage/inmemmory.py b/splitio/storage/inmemmory.py index 694279c9..9c36ab60 100644 --- a/splitio/storage/inmemmory.py +++ b/splitio/storage/inmemmory.py @@ -49,7 +49,22 @@ def fetch_many(self, split_names): """ return {split_name: self.get(split_name) for split_name in split_names} - def put(self, split): + def update(self, to_add, to_delete, new_change_number): + """ + Update feature flag strage. + + :param to_add: List of feature flags to add + :type to_add: list[splitio.models.splits.Split] + :param to_delete: List of feature flags to delete + :type to_delete: list[str] + :param new_change_number: New change number. + :type new_change_number: int + """ + [self._put(add_split) for add_split in to_add] + [self._remove(delete_split) for delete_split in to_delete] + self._set_change_number(new_change_number) + + def _put(self, split): """ Store a split. @@ -68,7 +83,7 @@ def put(self, split): self._sets_feature_flag_map[flag_set] = set() self._sets_feature_flag_map[flag_set].add(split.name) - def remove(self, split_name): + def _remove(self, split_name): """ Remove a split from storage. @@ -126,7 +141,7 @@ def get_change_number(self): with self._lock: return self._change_number - def set_change_number(self, new_change_number): + def _set_change_number(self, new_change_number): """ Set the latest change number. @@ -196,7 +211,7 @@ def kill_locally(self, feature_flag_name, default_treatment, change_number): if not split: return split.local_kill(default_treatment, change_number) - self.put(split) + self._put(split) def _increase_traffic_type_count(self, traffic_type_name): """ diff --git a/splitio/storage/pluggable.py b/splitio/storage/pluggable.py index a15df284..85af5ae4 100644 --- a/splitio/storage/pluggable.py +++ b/splitio/storage/pluggable.py @@ -87,7 +87,21 @@ def fetch_many(self, split_names): # _LOGGER.error('Error storing splits in storage') # _LOGGER.debug('Error: ', exc_info=True) - def remove(self, split_name): + def update(self, to_add, to_delete, new_change_number): + """ + Update feature flag strage. + + :param to_add: List of feature flags to add + :type to_add: list[splitio.models.splits.Split] + :param to_delete: List of feature flags to delete + :type to_delete: list[splitio.models.splits.Split] + :param new_change_number: New change number. + :type new_change_number: int + """ + pass + + # TODO: To be added when producer mode is aupported +# def _remove(self, split_name): """ Remove a split from storage. @@ -97,8 +111,7 @@ def remove(self, split_name): :return: True if the split was found and removed. False otherwise. :rtype: bool """ - pass - # TODO: To be added when producer mode is aupported +# pass # try: # split = self.get(split_name) # if not split: @@ -125,15 +138,15 @@ def get_change_number(self): _LOGGER.debug('Error: ', exc_info=True) return None - def set_change_number(self, new_change_number): + # TODO: To be added when producer mode is aupported +# def _set_change_number(self, new_change_number): """ Set the latest change number. :param new_change_number: New change number. :type new_change_number: int """ - pass - # TODO: To be added when producer mode is aupported +# pass # try: # self._pluggable_adapter.set(self._split_till_prefix, new_change_number) # except Exception: @@ -280,15 +293,15 @@ def is_valid_traffic_type(self, traffic_type_name): _LOGGER.debug('Error: ', exc_info=True) return None - def put(self, split): + # TODO: To be added when producer mode is aupported +# def _put(self, split): """ Store a split. :param split: Split object. :type split: splitio.models.split.Split """ - pass - # TODO: To be added when producer mode is aupported +# pass # try: # existing_split = self.get(split.name) # self._pluggable_adapter.set(self._prefix.format(split_name=split.name), split.to_json()) diff --git a/splitio/storage/redis.py b/splitio/storage/redis.py index d2aa2788..aa0e670c 100644 --- a/splitio/storage/redis.py +++ b/splitio/storage/redis.py @@ -128,24 +128,16 @@ def is_valid_traffic_type(self, traffic_type_name): # pylint: disable=method-hi _LOGGER.debug('Error: ', exc_info=True) return False - def put(self, split): + def update(self, to_add, to_delete, new_change_number): """ - Store a split. - - :param split: Split object to store - :type split_name: splitio.models.splits.Split - """ - raise NotImplementedError('Only redis-consumer mode is supported.') - - def remove(self, split_name): - """ - Remove a split from storage. - - :param split_name: Name of the feature to remove. - :type split_name: str + Update feature flag strage. - :return: True if the split was found and removed. False otherwise. - :rtype: bool + :param to_add: List of feature flags to add + :type to_add: list[splitio.models.splits.Split] + :param to_delete: List of feature flags to delete + :type to_delete: list[splitio.models.splits.Split] + :param new_change_number: New change number. + :type new_change_number: int """ raise NotImplementedError('Only redis-consumer mode is supported.') @@ -164,15 +156,6 @@ def get_change_number(self): _LOGGER.debug('Error: ', exc_info=True) return None - def set_change_number(self, new_change_number): - """ - Set the latest change number. - - :param new_change_number: New change number. - :type new_change_number: int - """ - raise NotImplementedError('Only redis-consumer mode is supported.') - def get_split_names(self): """ Retrieve a list of all split names. diff --git a/tests/storage/test_inmemory_storage.py b/tests/storage/test_inmemory_storage.py index 5c0352ac..061159d4 100644 --- a/tests/storage/test_inmemory_storage.py +++ b/tests/storage/test_inmemory_storage.py @@ -28,13 +28,14 @@ def test_storing_retrieving_splits(self, mocker): sets_property.return_value = None type(split).sets = sets_property - storage.put(split) + storage.update([split], [], 0) + assert storage.get('some_split') == split assert storage.get_split_names() == ['some_split'] assert storage.get_all_splits() == [split] assert storage.get('nonexistant_split') is None - storage.remove('some_split') + storage.update([], ['some_split'], 0) assert storage.get('some_split') is None def test_get_splits(self, mocker): @@ -53,8 +54,7 @@ def test_get_splits(self, mocker): type(split2).sets = sets_property storage = InMemorySplitStorage() - storage.put(split1) - storage.put(split2) + storage.update([split1, split2], [], 0) splits = storage.fetch_many(['split1', 'split2', 'split3']) assert len(splits) == 3 @@ -66,7 +66,7 @@ def test_store_get_changenumber(self): """Test that storing and retrieving change numbers works.""" storage = InMemorySplitStorage() assert storage.get_change_number() == -1 - storage.set_change_number(5) + storage.update([], [], 5) assert storage.get_change_number() == 5 def test_get_split_names(self, mocker): @@ -85,8 +85,7 @@ def test_get_split_names(self, mocker): type(split2).sets = sets_property storage = InMemorySplitStorage() - storage.put(split1) - storage.put(split2) + storage.update([split1, split2], [], 0) assert set(storage.get_split_names()) == set(['split1', 'split2']) @@ -106,8 +105,7 @@ def test_get_all_splits(self, mocker): type(split2).sets = sets_property storage = InMemorySplitStorage() - storage.put(split1) - storage.put(split2) + storage.update([split1, split2], [], 0) all_splits = storage.get_all_splits() assert next(s for s in all_splits if s.name == 'split1') @@ -142,27 +140,23 @@ def test_is_valid_traffic_type(self, mocker): storage = InMemorySplitStorage() - storage.put(split1) + storage.update([split1], [], 0) assert storage.is_valid_traffic_type('user') is True assert storage.is_valid_traffic_type('account') is False - storage.put(split2) - assert storage.is_valid_traffic_type('user') is True - assert storage.is_valid_traffic_type('account') is True - - storage.put(split3) + storage.update([split2, split3], [], 0) assert storage.is_valid_traffic_type('user') is True assert storage.is_valid_traffic_type('account') is True - storage.remove('split1') + storage.update([], ['split1'], 0) assert storage.is_valid_traffic_type('user') is True assert storage.is_valid_traffic_type('account') is True - storage.remove('split2') + storage.update([], ['split2'], 0) assert storage.is_valid_traffic_type('user') is True assert storage.is_valid_traffic_type('account') is False - storage.remove('split3') + storage.update([], ['split3'], 0) assert storage.is_valid_traffic_type('user') is False assert storage.is_valid_traffic_type('account') is False @@ -192,11 +186,11 @@ def test_traffic_type_inc_dec_logic(self, mocker): type(split1).traffic_type_name = tt_user type(split2).traffic_type_name = tt_account - storage.put(split1) + storage.update([split1], [], 0) assert storage.is_valid_traffic_type('user') is True assert storage.is_valid_traffic_type('account') is False - storage.put(split2) + storage.update([split2], [], 0) assert storage.is_valid_traffic_type('user') is False assert storage.is_valid_traffic_type('account') is True @@ -206,8 +200,7 @@ def test_kill_locally(self): split = Split('some_split', 123456789, False, 'some', 'traffic_type', 'ACTIVE', 1) - storage.put(split) - storage.set_change_number(1) + storage.update([split], [], 1) storage.kill_locally('test', 'default_treatment', 2) assert storage.get('test') is None @@ -226,28 +219,28 @@ def test_flag_sets(self): split1 = Split('split1', 123456789, False, 'some', 'traffic_type', 'ACTIVE', 1, sets=['set10', 'set02']) - storage.put(split1) + split2 = Split('split2', 123456789, False, 'some', 'traffic_type', + 'ACTIVE', 1, sets=['set05', 'set02']) + storage.update([split1], [], 1) assert storage.get_feature_flags_by_set('set10') == ['split1'] assert storage.get_feature_flags_by_set('set02') == ['split1'] - split2 = Split('split2', 123456789, False, 'some', 'traffic_type', - 'ACTIVE', 1, sets=['set05', 'set02']) - storage.put(split2) + storage.update([split2], [], 1) assert storage.get_feature_flags_by_set('set05') == ['split2'] assert sorted(storage.get_feature_flags_by_set('set02')) == ['split1', 'split2'] - storage.remove(split2.name) + storage.update([], [split2.name], 1) assert 'set5' not in storage._sets_feature_flag_map assert storage.get_feature_flags_by_set('set02') == ['split1'] assert storage.get_feature_flags_by_set('set05') == [] split1 = Split('split1', 123456789, False, 'some', 'traffic_type', 'ACTIVE', 1, sets=['set02']) - storage.put(split1) + storage.update([split1], [], 1) assert 'set10' not in storage._sets_feature_flag_map assert storage.get_feature_flags_by_set('set02') == ['split1'] - storage.remove(split1.name) + storage.update([], [split1.name], 1) assert storage._sets_feature_flag_map == {} assert storage.get_feature_flags_by_set('set02') == [] From 0bf33de77f7057de5c436f9102ffb3d163bb94bb Mon Sep 17 00:00:00 2001 From: Bilal Al-Shahwany <41021307+chillaq@users.noreply.github.com> Date: Tue, 29 Aug 2023 08:07:25 -0700 Subject: [PATCH 2/5] Update splitio/storage/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Thea --- splitio/storage/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/splitio/storage/__init__.py b/splitio/storage/__init__.py index a701ac8e..4930a95e 100644 --- a/splitio/storage/__init__.py +++ b/splitio/storage/__init__.py @@ -32,7 +32,7 @@ def fetch_many(self, split_names): @abc.abstractmethod def update(self, to_add, to_delete, new_change_number): """ - Update feature flag strage. + Update feature flag storage. :param to_add: List of feature flags to add :type to_add: list[splitio.models.splits.Split] From 06de1c915fcc24741a58e4c7a18ac32c5b987209 Mon Sep 17 00:00:00 2001 From: Bilal Al-Shahwany <41021307+chillaq@users.noreply.github.com> Date: Tue, 29 Aug 2023 08:07:33 -0700 Subject: [PATCH 3/5] Update splitio/storage/inmemmory.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Thea --- splitio/storage/inmemmory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/splitio/storage/inmemmory.py b/splitio/storage/inmemmory.py index 9c36ab60..746fdb61 100644 --- a/splitio/storage/inmemmory.py +++ b/splitio/storage/inmemmory.py @@ -51,7 +51,7 @@ def fetch_many(self, split_names): def update(self, to_add, to_delete, new_change_number): """ - Update feature flag strage. + Update feature flag storage. :param to_add: List of feature flags to add :type to_add: list[splitio.models.splits.Split] From ebb9e6838a161cf12fa3979067660db593ff6d86 Mon Sep 17 00:00:00 2001 From: Bilal Al-Shahwany <41021307+chillaq@users.noreply.github.com> Date: Tue, 29 Aug 2023 08:07:40 -0700 Subject: [PATCH 4/5] Update splitio/storage/pluggable.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Thea --- splitio/storage/pluggable.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/splitio/storage/pluggable.py b/splitio/storage/pluggable.py index 85af5ae4..2be0f6d3 100644 --- a/splitio/storage/pluggable.py +++ b/splitio/storage/pluggable.py @@ -89,7 +89,7 @@ def fetch_many(self, split_names): def update(self, to_add, to_delete, new_change_number): """ - Update feature flag strage. + Update feature flag storage. :param to_add: List of feature flags to add :type to_add: list[splitio.models.splits.Split] From 954465b0fbcd3a9dc17f7c194ed6d7b2a80d7af5 Mon Sep 17 00:00:00 2001 From: Bilal Al-Shahwany <41021307+chillaq@users.noreply.github.com> Date: Tue, 29 Aug 2023 08:07:45 -0700 Subject: [PATCH 5/5] Update splitio/storage/redis.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Thea --- splitio/storage/redis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/splitio/storage/redis.py b/splitio/storage/redis.py index aa0e670c..9433fdd4 100644 --- a/splitio/storage/redis.py +++ b/splitio/storage/redis.py @@ -130,7 +130,7 @@ def is_valid_traffic_type(self, traffic_type_name): # pylint: disable=method-hi def update(self, to_add, to_delete, new_change_number): """ - Update feature flag strage. + Update feature flag storage. :param to_add: List of feature flags to add :type to_add: list[splitio.models.splits.Split]