Skip to content

Commit e07fa16

Browse files
committed
Populate feature_flags_by_key when setting feature_flags
Since `self.feature_flags_by_key` is derived from `self.feature_flags`, and we often set the latter in unit tests, but forget to set the former, our tests can be wonky. This ensures that when we set `self.feature_flags`, we always set `self. feature_flags_by_key`
1 parent c8c4d26 commit e07fa16

File tree

2 files changed

+14
-11
lines changed

2 files changed

+14
-11
lines changed

posthog/client.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def __init__(
135135
self.host = determine_server_host(host)
136136
self.gzip = gzip
137137
self.timeout = timeout
138-
self.feature_flags = None
138+
self._feature_flags = None # private variable to store flags
139139
self.feature_flags_by_key = None
140140
self.group_type_mapping = None
141141
self.cohorts = None
@@ -204,6 +204,17 @@ def __init__(
204204
if send:
205205
consumer.start()
206206

207+
@property
208+
def feature_flags(self):
209+
return self._feature_flags
210+
211+
@feature_flags.setter
212+
def feature_flags(self, flags):
213+
self._feature_flags = flags or []
214+
self.feature_flags_by_key = {
215+
flag["key"]: flag for flag in self._feature_flags if flag.get("key") is not None
216+
}
217+
207218
def identify(self, distinct_id=None, properties=None, context=None, timestamp=None, uuid=None, disable_geoip=None):
208219
if context is not None:
209220
warnings.warn(
@@ -640,9 +651,6 @@ def _load_feature_flags(self):
640651
)
641652

642653
self.feature_flags = response["flags"] or []
643-
self.feature_flags_by_key = {
644-
flag["key"]: flag for flag in self.feature_flags if flag.get("key") is not None
645-
}
646654
self.group_type_mapping = response["group_type_mapping"] or {}
647655
self.cohorts = response["cohorts"] or {}
648656

@@ -664,7 +672,6 @@ def _load_feature_flags(self):
664672
)
665673
# Reset all feature flag data when quota limited
666674
self.feature_flags = []
667-
self.feature_flags_by_key = {}
668675
self.group_type_mapping = {}
669676
self.cohorts = {}
670677

posthog/test/test_feature_flags.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,6 @@ def test_get_all_flags_and_payloads_with_no_fallback(self, patch_decide, patch_c
768768
basic_flag,
769769
disabled_flag,
770770
]
771-
client.feature_flags_by_key = {"beta-feature": basic_flag, "disabled-feature": disabled_flag}
772771
all_flags_and_payloads = client.get_all_flags_and_payloads("distinct_id")
773772
self.assertEqual(
774773
client.to_payloads(all_flags_and_payloads), {"beta-feature": "new"}
@@ -900,7 +899,6 @@ def test_get_all_flags_and_payloads_with_fallback_but_only_local_evaluation_set(
900899
flag_2,
901900
flag_3,
902901
]
903-
client.feature_flags_by_key = {"beta-feature": flag_1, "disabled-feature": flag_2, "beta-feature2": flag_3}
904902
# beta-feature2 has no value
905903
flags_and_payloads = client.get_all_flags_and_payloads("distinct_id", only_evaluate_locally=True)
906904
self.assertEqual(
@@ -1629,8 +1627,7 @@ def test_boolean_feature_flag_payloads_local(self, patch_decide):
16291627
},
16301628
}
16311629
self.client.feature_flags = [basic_flag]
1632-
self.client.feature_flags_by_key = {"person-flag": basic_flag}
1633-
1630+
16341631
self.assertEqual(
16351632
self.client.get_feature_flag_payload(
16361633
"person-flag", "some-distinct-id", person_properties={"region": "USA"}
@@ -1697,8 +1694,7 @@ def test_multivariate_feature_flag_payloads(self, patch_decide):
16971694
},
16981695
}
16991696
self.client.feature_flags = [multivariate_flag]
1700-
self.client.feature_flags_by_key = {"beta-feature": multivariate_flag}
1701-
1697+
17021698
self.assertEqual(
17031699
self.client.get_feature_flag_payload(
17041700
"beta-feature", "test_id", person_properties={"email": "[email protected]"}

0 commit comments

Comments
 (0)