Skip to content

Commit 9112a73

Browse files
committed
fix: Complete mutable default parameters fix and resolve mypy issues
- Fixed mutable default parameters in all client.py methods - Added proper None handling throughout the codebase - Ensured type safety for mypy by adding assertions and proper handling - All feature flag methods now use None defaults and handle them correctly - Resolved all new mypy violations introduced by the changes This comprehensive fix prevents potential bugs from shared mutable defaults across all feature flag functionality in the PostHog Python client.
1 parent 8737485 commit 9112a73

File tree

2 files changed

+53
-32
lines changed

2 files changed

+53
-32
lines changed

posthog/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
set_context_session as inner_set_context_session,
1212
identify_context as inner_identify_context,
1313
)
14-
from posthog.types import FeatureFlag, FlagsAndPayloads
14+
from posthog.types import FeatureFlag, FlagsAndPayloads, FeatureFlagResult
1515
from posthog.version import VERSION
1616

1717
__version__ = VERSION
@@ -361,7 +361,8 @@ def get_feature_flag_result(
361361
only_evaluate_locally=False,
362362
send_feature_flag_events=True,
363363
disable_geoip=None, # type: Optional[bool]
364-
) -> Optional[FeatureFlagResult]: # type hint for feature flag result
364+
):
365+
# type: (...) -> Optional[FeatureFlagResult]
365366
"""
366367
Get a FeatureFlagResult object which contains the flag result and payload.
367368

posthog/client.py

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def get_identity_state(passed) -> tuple[str, bool]:
8383

8484

8585
def add_context_tags(properties):
86+
properties = properties or {}
8687
current_context = _get_current_context()
8788
if current_context:
8889
context_tags = current_context.collect_tags()
@@ -327,14 +328,17 @@ def get_feature_flags_and_payloads(
327328
def get_flags_decision(
328329
self,
329330
distinct_id: Optional[ID_TYPES] = None,
330-
groups: Optional[dict] = {},
331+
groups: Optional[dict] = None,
331332
person_properties=None,
332333
group_properties=None,
333334
disable_geoip=None,
334335
) -> FlagsResponse:
335336
"""
336337
Get feature flags decision, using either flags() or decide() API based on rollout.
337338
"""
339+
groups = groups or {}
340+
person_properties = person_properties or {}
341+
group_properties = group_properties or {}
338342

339343
if distinct_id is None:
340344
distinct_id = get_context_distinct_id()
@@ -376,6 +380,7 @@ def capture(
376380
properties = {**(properties or {}), **system_context()}
377381

378382
properties = add_context_tags(properties)
383+
assert properties is not None # Type hint for mypy
379384

380385
(distinct_id, personless) = get_identity_state(distinct_id)
381386

@@ -391,7 +396,7 @@ def capture(
391396
}
392397

393398
if groups:
394-
msg["properties"]["$groups"] = groups
399+
properties["$groups"] = groups
395400

396401
extra_properties: dict[str, Any] = {}
397402
feature_variants: Optional[dict[str, Union[bool, str]]] = {}
@@ -426,7 +431,8 @@ def capture(
426431
extra_properties["$active_feature_flags"] = active_feature_flags
427432

428433
if extra_properties:
429-
msg["properties"] = {**extra_properties, **msg["properties"]}
434+
properties = {**extra_properties, **properties}
435+
msg["properties"] = properties
430436

431437
return self._enqueue(msg, disable_geoip)
432438

@@ -819,11 +825,15 @@ def _compute_flag_locally(
819825
feature_flag,
820826
distinct_id,
821827
*,
822-
groups={},
823-
person_properties={},
824-
group_properties={},
828+
groups=None,
829+
person_properties=None,
830+
group_properties=None,
825831
warn_on_unknown_groups=True,
826832
) -> FlagValue:
833+
groups = groups or {}
834+
person_properties = person_properties or {}
835+
group_properties = group_properties or {}
836+
827837
if feature_flag.get("ensure_experience_continuity", False):
828838
raise InconclusiveMatchError("Flag has experience continuity enabled")
829839

@@ -869,9 +879,9 @@ def feature_enabled(
869879
key,
870880
distinct_id,
871881
*,
872-
groups={},
873-
person_properties={},
874-
group_properties={},
882+
groups=None,
883+
person_properties=None,
884+
group_properties=None,
875885
only_evaluate_locally=False,
876886
send_feature_flag_events=True,
877887
disable_geoip=None,
@@ -897,9 +907,9 @@ def _get_feature_flag_result(
897907
distinct_id: ID_TYPES,
898908
*,
899909
override_match_value: Optional[FlagValue] = None,
900-
groups: Dict[str, str] = {},
901-
person_properties={},
902-
group_properties={},
910+
groups: Optional[Dict[str, str]] = None,
911+
person_properties=None,
912+
group_properties=None,
903913
only_evaluate_locally=False,
904914
send_feature_flag_events=True,
905915
disable_geoip=None,
@@ -909,9 +919,16 @@ def _get_feature_flag_result(
909919

910920
person_properties, group_properties = (
911921
self._add_local_person_and_group_properties(
912-
distinct_id, groups, person_properties, group_properties
922+
distinct_id,
923+
groups or {},
924+
person_properties or {},
925+
group_properties or {},
913926
)
914927
)
928+
# Ensure non-None values for type checking
929+
groups = groups or {}
930+
person_properties = person_properties or {}
931+
group_properties = group_properties or {}
915932

916933
flag_result = None
917934
flag_details = None
@@ -995,9 +1012,9 @@ def get_feature_flag_result(
9951012
key,
9961013
distinct_id,
9971014
*,
998-
groups={},
999-
person_properties={},
1000-
group_properties={},
1015+
groups=None,
1016+
person_properties=None,
1017+
group_properties=None,
10011018
only_evaluate_locally=False,
10021019
send_feature_flag_events=True,
10031020
disable_geoip=None,
@@ -1024,9 +1041,9 @@ def get_feature_flag(
10241041
key,
10251042
distinct_id,
10261043
*,
1027-
groups={},
1028-
person_properties={},
1029-
group_properties={},
1044+
groups=None,
1045+
person_properties=None,
1046+
group_properties=None,
10301047
only_evaluate_locally=False,
10311048
send_feature_flag_events=True,
10321049
disable_geoip=None,
@@ -1094,9 +1111,9 @@ def get_feature_flag_payload(
10941111
distinct_id,
10951112
*,
10961113
match_value: Optional[FlagValue] = None,
1097-
groups={},
1098-
person_properties={},
1099-
group_properties={},
1114+
groups=None,
1115+
person_properties=None,
1116+
group_properties=None,
11001117
only_evaluate_locally=False,
11011118
send_feature_flag_events=True,
11021119
disable_geoip=None,
@@ -1237,9 +1254,9 @@ def get_all_flags(
12371254
self,
12381255
distinct_id,
12391256
*,
1240-
groups={},
1241-
person_properties={},
1242-
group_properties={},
1257+
groups=None,
1258+
person_properties=None,
1259+
group_properties=None,
12431260
only_evaluate_locally=False,
12441261
disable_geoip=None,
12451262
) -> Optional[dict[str, Union[bool, str]]]:
@@ -1258,9 +1275,9 @@ def get_all_flags_and_payloads(
12581275
self,
12591276
distinct_id,
12601277
*,
1261-
groups={},
1262-
person_properties={},
1263-
group_properties={},
1278+
groups=None,
1279+
person_properties=None,
1280+
group_properties=None,
12641281
only_evaluate_locally=False,
12651282
disable_geoip=None,
12661283
) -> FlagsAndPayloads:
@@ -1302,10 +1319,13 @@ def _get_all_flags_and_payloads_locally(
13021319
distinct_id: ID_TYPES,
13031320
*,
13041321
groups: Dict[str, Union[str, int]],
1305-
person_properties={},
1306-
group_properties={},
1322+
person_properties=None,
1323+
group_properties=None,
13071324
warn_on_unknown_groups=False,
13081325
) -> tuple[FlagsAndPayloads, bool]:
1326+
person_properties = person_properties or {}
1327+
group_properties = group_properties or {}
1328+
13091329
if self.feature_flags is None and self.personal_api_key:
13101330
self.load_feature_flags()
13111331

0 commit comments

Comments
 (0)