Skip to content

Commit

Permalink
fix(flags): Update relative date op names (#113)
Browse files Browse the repository at this point in the history
  • Loading branch information
neilkakkar authored Jan 26, 2024
1 parent d0d962a commit 8554b51
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 41 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 3.3.3 - 2024-01-26

1. Remove new relative date operators, combine into regular date operators

## 3.3.2 - 2024-01-19

1. Return success/failure with all capture calls from module functions
Expand Down
29 changes: 23 additions & 6 deletions posthog/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,16 @@ def load_feature_flags(self):
self.poller = Poller(interval=timedelta(seconds=self.poll_interval), execute=self._load_feature_flags)
self.poller.start()

def _compute_flag_locally(self, feature_flag, distinct_id, *, groups={}, person_properties={}, group_properties={}):
def _compute_flag_locally(
self,
feature_flag,
distinct_id,
*,
groups={},
person_properties={},
group_properties={},
warn_on_unknown_groups=True,
):
if feature_flag.get("ensure_experience_continuity", False):
raise InconclusiveMatchError("Flag has experience continuity enabled")

Expand All @@ -486,9 +495,14 @@ def _compute_flag_locally(self, feature_flag, distinct_id, *, groups={}, person_
if group_name not in groups:
# Group flags are never enabled in `groups` aren't passed in
# don't failover to `/decide/`, since response will be the same
self.log.warning(
f"[FEATURE FLAGS] Can't compute group feature flag: {feature_flag['key']} without group names passed in"
)
if warn_on_unknown_groups:
self.log.warning(
f"[FEATURE FLAGS] Can't compute group feature flag: {feature_flag['key']} without group names passed in"
)
else:
self.log.debug(
f"[FEATURE FLAGS] Can't compute group feature flag: {feature_flag['key']} without group names passed in"
)
return False

focused_group_properties = group_properties[group_name]
Expand Down Expand Up @@ -717,7 +731,9 @@ def get_all_flags_and_payloads(

return response

def _get_all_flags_and_payloads_locally(self, distinct_id, *, groups={}, person_properties={}, group_properties={}):
def _get_all_flags_and_payloads_locally(
self, distinct_id, *, groups={}, person_properties={}, group_properties={}, warn_on_unknown_groups=False
):
require("distinct_id", distinct_id, ID_TYPES)
require("groups", groups, dict)

Expand All @@ -737,6 +753,7 @@ def _get_all_flags_and_payloads_locally(self, distinct_id, *, groups={}, person_
groups=groups,
person_properties=person_properties,
group_properties=group_properties,
warn_on_unknown_groups=warn_on_unknown_groups,
)
matched_payload = self._compute_payload_locally(flag["key"], flags[flag["key"]])
if matched_payload:
Expand All @@ -756,7 +773,7 @@ def feature_flag_definitions(self):
return self.feature_flags

def _add_local_person_and_group_properties(self, distinct_id, groups, person_properties, group_properties):
all_person_properties = {"$current_distinct_id": distinct_id, **(person_properties or {})}
all_person_properties = {"distinct_id": distinct_id, **(person_properties or {})}

all_group_properties = {}
if groups:
Expand Down
16 changes: 8 additions & 8 deletions posthog/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,11 @@ def compare(lhs, rhs, operator):
else:
return compare(str(override_value), str(value), operator)

if operator in ["is_date_before", "is_date_after", "is_relative_date_before", "is_relative_date_after"]:
if operator in ["is_date_before", "is_date_after"]:
try:
if operator in ["is_relative_date_before", "is_relative_date_after"]:
parsed_date = relative_date_parse_for_feature_flag_matching(str(value))
else:
parsed_date = relative_date_parse_for_feature_flag_matching(str(value))

if not parsed_date:
parsed_date = parser.parse(str(value))
parsed_date = convert_to_datetime_aware(parsed_date)
except Exception as e:
Expand All @@ -190,20 +190,20 @@ def compare(lhs, rhs, operator):

if isinstance(override_value, datetime.datetime):
override_date = convert_to_datetime_aware(override_value)
if operator in ("is_date_before", "is_relative_date_before"):
if operator == "is_date_before":
return override_date < parsed_date
else:
return override_date > parsed_date
elif isinstance(override_value, datetime.date):
if operator in ("is_date_before", "is_relative_date_before"):
if operator == "is_date_before":
return override_value < parsed_date.date()
else:
return override_value > parsed_date.date()
elif isinstance(override_value, str):
try:
override_date = parser.parse(override_value)
override_date = convert_to_datetime_aware(override_date)
if operator in ("is_date_before", "is_relative_date_before"):
if operator == "is_date_before":
return override_date < parsed_date
else:
return override_date > parsed_date
Expand Down Expand Up @@ -302,7 +302,7 @@ def match_property_group(property_group, property_values, cohort_properties) ->


def relative_date_parse_for_feature_flag_matching(value: str) -> Optional[datetime.datetime]:
regex = r"^(?P<number>[0-9]+)(?P<interval>[a-z])$"
regex = r"^-?(?P<number>[0-9]+)(?P<interval>[a-z])$"
match = re.search(regex, value)
parsed_dt = datetime.datetime.now(datetime.timezone.utc)
if match:
Expand Down
6 changes: 3 additions & 3 deletions posthog/sentry/posthog_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ def processor(event, hint):
not not Hub.current.client.dsn and Dsn(Hub.current.client.dsn).project_id
)
if project_id:
properties[
"$sentry_url"
] = f"{PostHogIntegration.prefix}{PostHogIntegration.organization}/issues/?project={project_id}&query={event['event_id']}"
properties["$sentry_url"] = (
f"{PostHogIntegration.prefix}{PostHogIntegration.organization}/issues/?project={project_id}&query={event['event_id']}"
)

posthog.capture(posthog_distinct_id, "$exception", properties)

Expand Down
14 changes: 7 additions & 7 deletions posthog/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ def test_disable_geoip_default_on_decide(self, patch_decide):
timeout=10,
distinct_id="some_id",
groups={},
person_properties={"$current_distinct_id": "some_id"},
person_properties={"distinct_id": "some_id"},
group_properties={},
disable_geoip=True,
)
Expand All @@ -795,7 +795,7 @@ def test_disable_geoip_default_on_decide(self, patch_decide):
timeout=10,
distinct_id="feature_enabled_distinct_id",
groups={},
person_properties={"$current_distinct_id": "feature_enabled_distinct_id"},
person_properties={"distinct_id": "feature_enabled_distinct_id"},
group_properties={},
disable_geoip=True,
)
Expand All @@ -807,7 +807,7 @@ def test_disable_geoip_default_on_decide(self, patch_decide):
timeout=10,
distinct_id="all_flags_payloads_id",
groups={},
person_properties={"$current_distinct_id": "all_flags_payloads_id"},
person_properties={"distinct_id": "all_flags_payloads_id"},
group_properties={},
disable_geoip=False,
)
Expand Down Expand Up @@ -843,7 +843,7 @@ def test_default_properties_get_added_properly(self, patch_decide):
timeout=10,
distinct_id="some_id",
groups={"company": "id:5", "instance": "app.posthog.com"},
person_properties={"$current_distinct_id": "some_id", "x1": "y1"},
person_properties={"distinct_id": "some_id", "x1": "y1"},
group_properties={
"company": {"$group_key": "id:5", "x": "y"},
"instance": {"$group_key": "app.posthog.com"},
Expand All @@ -856,7 +856,7 @@ def test_default_properties_get_added_properly(self, patch_decide):
"random_key",
"some_id",
groups={"company": "id:5", "instance": "app.posthog.com"},
person_properties={"$current_distinct_id": "override"},
person_properties={"distinct_id": "override"},
group_properties={
"company": {
"$group_key": "group_override",
Expand All @@ -869,7 +869,7 @@ def test_default_properties_get_added_properly(self, patch_decide):
timeout=10,
distinct_id="some_id",
groups={"company": "id:5", "instance": "app.posthog.com"},
person_properties={"$current_distinct_id": "override"},
person_properties={"distinct_id": "override"},
group_properties={
"company": {"$group_key": "group_override"},
"instance": {"$group_key": "app.posthog.com"},
Expand All @@ -886,7 +886,7 @@ def test_default_properties_get_added_properly(self, patch_decide):
timeout=10,
distinct_id="some_id",
groups={},
person_properties={"$current_distinct_id": "some_id"},
person_properties={"distinct_id": "some_id"},
group_properties={},
disable_geoip=False,
)
32 changes: 16 additions & 16 deletions posthog/test/test_feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -1873,7 +1873,7 @@ def test_match_property_date_operators(self):

@freeze_time("2022-05-01")
def test_match_property_relative_date_operators(self):
property_a = self.property(key="key", value="6h", operator="is_relative_date_before")
property_a = self.property(key="key", value="-6h", operator="is_date_before")
self.assertTrue(match_property(property_a, {"key": "2022-03-01"}))
self.assertTrue(match_property(property_a, {"key": "2022-04-30"}))
self.assertTrue(match_property(property_a, {"key": datetime.datetime(2022, 4, 30, 1, 2, 3)}))
Expand All @@ -1898,7 +1898,7 @@ def test_match_property_relative_date_operators(self):
with self.assertRaises(InconclusiveMatchError):
match_property(property_a, {"key": "abcdef"})

property_b = self.property(key="key", value="1h", operator="is_relative_date_after")
property_b = self.property(key="key", value="1h", operator="is_date_after")
self.assertTrue(match_property(property_b, {"key": "2022-05-02"}))
self.assertTrue(match_property(property_b, {"key": "2022-05-30"}))
self.assertTrue(match_property(property_b, {"key": datetime.datetime(2022, 5, 30)}))
Expand All @@ -1910,16 +1910,16 @@ def test_match_property_relative_date_operators(self):
self.assertFalse(match_property(property_b, {"key": "abcdef"}))

# Invalid flag property
property_c = self.property(key="key", value=1234, operator="is_relative_date_after")
property_c = self.property(key="key", value=1234, operator="is_date_after")

with self.assertRaises(InconclusiveMatchError):
self.assertFalse(match_property(property_c, {"key": 1}))

with self.assertRaises(InconclusiveMatchError):
self.assertFalse(match_property(property_c, {"key": "2022-05-30"}))
# parsed as 1234-05-01 for some reason?
self.assertTrue(match_property(property_c, {"key": "2022-05-30"}))

# # Timezone aware property
property_d = self.property(key="key", value="12d", operator="is_relative_date_before")
property_d = self.property(key="key", value="12d", operator="is_date_before")
self.assertFalse(match_property(property_d, {"key": "2022-05-30"}))

self.assertTrue(match_property(property_d, {"key": "2022-03-30"}))
Expand All @@ -1929,45 +1929,45 @@ def test_match_property_relative_date_operators(self):
self.assertFalse(match_property(property_d, {"key": "2022-04-19 02:00:01+02:00"}))

# Try all possible relative dates
property_e = self.property(key="key", value="1h", operator="is_relative_date_before")
property_e = self.property(key="key", value="1h", operator="is_date_before")
self.assertFalse(match_property(property_e, {"key": "2022-05-01 00:00:00"}))
self.assertTrue(match_property(property_e, {"key": "2022-04-30 22:00:00"}))

property_f = self.property(key="key", value="1d", operator="is_relative_date_before")
property_f = self.property(key="key", value="-1d", operator="is_date_before")
self.assertTrue(match_property(property_f, {"key": "2022-04-29 23:59:00"}))
self.assertFalse(match_property(property_f, {"key": "2022-04-30 00:00:01"}))

property_g = self.property(key="key", value="1w", operator="is_relative_date_before")
property_g = self.property(key="key", value="1w", operator="is_date_before")
self.assertTrue(match_property(property_g, {"key": "2022-04-23 00:00:00"}))
self.assertFalse(match_property(property_g, {"key": "2022-04-24 00:00:00"}))
self.assertFalse(match_property(property_g, {"key": "2022-04-24 00:00:01"}))

property_h = self.property(key="key", value="1m", operator="is_relative_date_before")
property_h = self.property(key="key", value="1m", operator="is_date_before")
self.assertTrue(match_property(property_h, {"key": "2022-03-01 00:00:00"}))
self.assertFalse(match_property(property_h, {"key": "2022-04-05 00:00:00"}))

property_i = self.property(key="key", value="1y", operator="is_relative_date_before")
property_i = self.property(key="key", value="1y", operator="is_date_before")
self.assertTrue(match_property(property_i, {"key": "2021-04-28 00:00:00"}))
self.assertFalse(match_property(property_i, {"key": "2021-05-01 00:00:01"}))

property_j = self.property(key="key", value="122h", operator="is_relative_date_after")
property_j = self.property(key="key", value="122h", operator="is_date_after")
self.assertTrue(match_property(property_j, {"key": "2022-05-01 00:00:00"}))
self.assertFalse(match_property(property_j, {"key": "2022-04-23 01:00:00"}))

property_k = self.property(key="key", value="2d", operator="is_relative_date_after")
property_k = self.property(key="key", value="2d", operator="is_date_after")
self.assertTrue(match_property(property_k, {"key": "2022-05-01 00:00:00"}))
self.assertTrue(match_property(property_k, {"key": "2022-04-29 00:00:01"}))
self.assertFalse(match_property(property_k, {"key": "2022-04-29 00:00:00"}))

property_l = self.property(key="key", value="02w", operator="is_relative_date_after")
property_l = self.property(key="key", value="-02w", operator="is_date_after")
self.assertTrue(match_property(property_l, {"key": "2022-05-01 00:00:00"}))
self.assertFalse(match_property(property_l, {"key": "2022-04-16 00:00:00"}))

property_m = self.property(key="key", value="1m", operator="is_relative_date_after")
property_m = self.property(key="key", value="1m", operator="is_date_after")
self.assertTrue(match_property(property_m, {"key": "2022-04-01 00:00:01"}))
self.assertFalse(match_property(property_m, {"key": "2022-04-01 00:00:00"}))

property_n = self.property(key="key", value="1y", operator="is_relative_date_after")
property_n = self.property(key="key", value="1y", operator="is_date_after")
self.assertTrue(match_property(property_n, {"key": "2022-05-01 00:00:00"}))
self.assertTrue(match_property(property_n, {"key": "2021-05-01 00:00:01"}))
self.assertFalse(match_property(property_n, {"key": "2021-05-01 00:00:00"}))
Expand Down
2 changes: 1 addition & 1 deletion posthog/version.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
VERSION = "3.3.2"
VERSION = "3.3.3"

if __name__ == "__main__":
print(VERSION, end="") # noqa: T201
1 change: 1 addition & 0 deletions sentry_django_example/sentry_django_example/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
1. Import the include() function: from django.urls import include, path
2. Add a URL to urlpatterns: path('blog/', include('blog.urls'))
"""

from django.contrib import admin
from django.urls import path

Expand Down

0 comments on commit 8554b51

Please sign in to comment.