Skip to content

Commit f9f63df

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 95b7df9 commit f9f63df

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
@@ -537,7 +537,8 @@ def get_feature_flag_result(
537537
only_evaluate_locally=False,
538538
send_feature_flag_events=True,
539539
disable_geoip=None, # type: Optional[bool]
540-
) -> Optional[FeatureFlagResult]: # type hint for feature flag result
540+
):
541+
# type: (...) -> Optional[FeatureFlagResult]
541542
"""
542543
Get a FeatureFlagResult object which contains the flag result and payload.
543544

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()
@@ -395,7 +396,7 @@ def get_feature_flags_and_payloads(
395396
def get_flags_decision(
396397
self,
397398
distinct_id: Optional[ID_TYPES] = None,
398-
groups: Optional[dict] = {},
399+
groups: Optional[dict] = None,
399400
person_properties=None,
400401
group_properties=None,
401402
disable_geoip=None,
@@ -418,6 +419,9 @@ def get_flags_decision(
418419
Category:
419420
Feature Flags
420421
"""
422+
groups = groups or {}
423+
person_properties = person_properties or {}
424+
group_properties = group_properties or {}
421425

422426
if distinct_id is None:
423427
distinct_id = get_context_distinct_id()
@@ -505,6 +509,7 @@ def capture(
505509
properties = {**(properties or {}), **system_context()}
506510

507511
properties = add_context_tags(properties)
512+
assert properties is not None # Type hint for mypy
508513

509514
(distinct_id, personless) = get_identity_state(distinct_id)
510515

@@ -520,7 +525,7 @@ def capture(
520525
}
521526

522527
if groups:
523-
msg["properties"]["$groups"] = groups
528+
properties["$groups"] = groups
524529

525530
extra_properties: dict[str, Any] = {}
526531
feature_variants: Optional[dict[str, Union[bool, str]]] = {}
@@ -575,7 +580,8 @@ def capture(
575580
extra_properties["$active_feature_flags"] = active_feature_flags
576581

577582
if extra_properties:
578-
msg["properties"] = {**extra_properties, **msg["properties"]}
583+
properties = {**extra_properties, **properties}
584+
msg["properties"] = properties
579585

580586
return self._enqueue(msg, disable_geoip)
581587

@@ -1153,11 +1159,15 @@ def _compute_flag_locally(
11531159
feature_flag,
11541160
distinct_id,
11551161
*,
1156-
groups={},
1157-
person_properties={},
1158-
group_properties={},
1162+
groups=None,
1163+
person_properties=None,
1164+
group_properties=None,
11591165
warn_on_unknown_groups=True,
11601166
) -> FlagValue:
1167+
groups = groups or {}
1168+
person_properties = person_properties or {}
1169+
group_properties = group_properties or {}
1170+
11611171
if feature_flag.get("ensure_experience_continuity", False):
11621172
raise InconclusiveMatchError("Flag has experience continuity enabled")
11631173

@@ -1203,9 +1213,9 @@ def feature_enabled(
12031213
key,
12041214
distinct_id,
12051215
*,
1206-
groups={},
1207-
person_properties={},
1208-
group_properties={},
1216+
groups=None,
1217+
person_properties=None,
1218+
group_properties=None,
12091219
only_evaluate_locally=False,
12101220
send_feature_flag_events=True,
12111221
disable_geoip=None,
@@ -1256,9 +1266,9 @@ def _get_feature_flag_result(
12561266
distinct_id: ID_TYPES,
12571267
*,
12581268
override_match_value: Optional[FlagValue] = None,
1259-
groups: Dict[str, str] = {},
1260-
person_properties={},
1261-
group_properties={},
1269+
groups: Optional[Dict[str, str]] = None,
1270+
person_properties=None,
1271+
group_properties=None,
12621272
only_evaluate_locally=False,
12631273
send_feature_flag_events=True,
12641274
disable_geoip=None,
@@ -1268,9 +1278,16 @@ def _get_feature_flag_result(
12681278

12691279
person_properties, group_properties = (
12701280
self._add_local_person_and_group_properties(
1271-
distinct_id, groups, person_properties, group_properties
1281+
distinct_id,
1282+
groups or {},
1283+
person_properties or {},
1284+
group_properties or {},
12721285
)
12731286
)
1287+
# Ensure non-None values for type checking
1288+
groups = groups or {}
1289+
person_properties = person_properties or {}
1290+
group_properties = group_properties or {}
12741291

12751292
flag_result = None
12761293
flag_details = None
@@ -1354,9 +1371,9 @@ def get_feature_flag_result(
13541371
key,
13551372
distinct_id,
13561373
*,
1357-
groups={},
1358-
person_properties={},
1359-
group_properties={},
1374+
groups=None,
1375+
person_properties=None,
1376+
group_properties=None,
13601377
only_evaluate_locally=False,
13611378
send_feature_flag_events=True,
13621379
disable_geoip=None,
@@ -1404,9 +1421,9 @@ def get_feature_flag(
14041421
key,
14051422
distinct_id,
14061423
*,
1407-
groups={},
1408-
person_properties={},
1409-
group_properties={},
1424+
groups=None,
1425+
person_properties=None,
1426+
group_properties=None,
14101427
only_evaluate_locally=False,
14111428
send_feature_flag_events=True,
14121429
disable_geoip=None,
@@ -1492,9 +1509,9 @@ def get_feature_flag_payload(
14921509
distinct_id,
14931510
*,
14941511
match_value: Optional[FlagValue] = None,
1495-
groups={},
1496-
person_properties={},
1497-
group_properties={},
1512+
groups=None,
1513+
person_properties=None,
1514+
group_properties=None,
14981515
only_evaluate_locally=False,
14991516
send_feature_flag_events=True,
15001517
disable_geoip=None,
@@ -1662,9 +1679,9 @@ def get_all_flags(
16621679
self,
16631680
distinct_id,
16641681
*,
1665-
groups={},
1666-
person_properties={},
1667-
group_properties={},
1682+
groups=None,
1683+
person_properties=None,
1684+
group_properties=None,
16681685
only_evaluate_locally=False,
16691686
disable_geoip=None,
16701687
) -> Optional[dict[str, Union[bool, str]]]:
@@ -1702,9 +1719,9 @@ def get_all_flags_and_payloads(
17021719
self,
17031720
distinct_id,
17041721
*,
1705-
groups={},
1706-
person_properties={},
1707-
group_properties={},
1722+
groups=None,
1723+
person_properties=None,
1724+
group_properties=None,
17081725
only_evaluate_locally=False,
17091726
disable_geoip=None,
17101727
) -> FlagsAndPayloads:
@@ -1765,10 +1782,13 @@ def _get_all_flags_and_payloads_locally(
17651782
distinct_id: ID_TYPES,
17661783
*,
17671784
groups: Dict[str, Union[str, int]],
1768-
person_properties={},
1769-
group_properties={},
1785+
person_properties=None,
1786+
group_properties=None,
17701787
warn_on_unknown_groups=False,
17711788
) -> tuple[FlagsAndPayloads, bool]:
1789+
person_properties = person_properties or {}
1790+
group_properties = group_properties or {}
1791+
17721792
if self.feature_flags is None and self.personal_api_key:
17731793
self.load_feature_flags()
17741794

0 commit comments

Comments
 (0)