Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(insights): Run all tests with filters-based insights using HogQL #21861

Merged
merged 19 commits into from
May 23, 2024
Merged

test(insights): Run all tests with filters-based insights using HogQL #21861

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3407f75
ci: Run core backend tests with both HogQL and legacy insights
Twixes Apr 25, 2024
adb4fe8
Double the number of tests
Twixes Apr 25, 2024
e8e59c6
Fix env var setting
Twixes Apr 25, 2024
1f2469c
Add some HOGQL_INSIGHTS_OVERRIDE overrides in tests
Twixes Apr 25, 2024
f15ecb9
Mark `Insight.last_refresh` as deprecated
Twixes Apr 25, 2024
dd9aa72
Fix bad merge
Twixes May 10, 2024
88fa51d
Update query snapshots
github-actions[bot] May 22, 2024
e5af07c
Update query snapshots
github-actions[bot] May 22, 2024
b83c6b9
Update test_team.py
Twixes May 22, 2024
d91cd20
Actually remove legacy backend from matrix
Twixes May 23, 2024
c32b377
Update test_fetch_from_cache.py
Twixes May 23, 2024
b28e699
Use `update_cached_state` in `calculate_for_query_based_insight()`
Twixes May 23, 2024
e1160c5
Clean up CI changes and a comment
Twixes May 23, 2024
483d786
Fix `update_cached_state` typing
Twixes May 23, 2024
1a8dffd
Update test_fetch_from_cache.py
Twixes May 23, 2024
ee7e7c0
Update test_insight_cache.py
Twixes May 23, 2024
8fa0b5f
Update test_insight_cache.py
Twixes May 23, 2024
0c0bb44
Clarify `generate_insight_cache_key` as legacy function
Twixes May 23, 2024
95cc8be
Update test_should_refresh_insight.py
Twixes May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/run-backend-tests/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ runs:
env:
PERSON_ON_EVENTS_V2_ENABLED: ${{ inputs.person-on-events }}
GROUPS_ON_EVENTS_ENABLED: ${{ inputs.person-on-events }}
HOGQL_INSIGHTS_OVERRIDE: '1' # HogQL insights are fully rolled out in production
shell: bash
run: | # async_migrations covered in ci-async-migrations.yml
pytest ${{
Expand Down
1 change: 0 additions & 1 deletion posthog/api/insight.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ def create(self, validated_data: dict, *args: Any, **kwargs: Any) -> Insight:
raise serializers.ValidationError("Dashboard not found")

DashboardTile.objects.create(insight=insight, dashboard=dashboard, last_refresh=now())
insight.last_refresh = now() # set last refresh if the insight is on at least one dashboard
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has not been used in a long time and not sure it ever actually worked. The actual last_refresh is in InsightCachingState at the insight level, and in the cached result packages themselves


# Manual tag creation since this create method doesn't call super()
self._attempt_set_tags(tags, insight)
Expand Down
9 changes: 6 additions & 3 deletions posthog/api/team.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,11 @@ def create(self, validated_data: dict[str, Any], **kwargs) -> Team:

return team

def _clear_team_insight_cache(self, team: Team) -> None:
# :KLUDGE: This is incorrect as it doesn't wipe caches not currently linked to insights. Fix this some day!
def _clear_team_insight_caching_states(self, team: Team) -> None:
# TODO: Remove this method:
# 1. It only clear the cache for saved insights, queries not linked to one are being ignored here
# 2. We should anyway 100% be relying on cache keys being different for materially different queries, instead of
# on remembering to call this method when project settings change. We probably already are in the clear here!
hashes = InsightCachingState.objects.filter(team=team).values_list("cache_key", flat=True)
cache.delete_many(hashes)

Expand All @@ -349,7 +352,7 @@ def update(self, instance: Team, validated_data: dict[str, Any]) -> Team:
if ("timezone" in validated_data and validated_data["timezone"] != instance.timezone) or (
"modifiers" in validated_data and validated_data["modifiers"] != instance.modifiers
):
self._clear_team_insight_cache(instance)
self._clear_team_insight_caching_states(instance)

if (
"session_replay_config" in validated_data
Expand Down
4 changes: 2 additions & 2 deletions posthog/api/test/__snapshots__/test_annotation.ambr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -220,6 +219,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down Expand Up @@ -619,7 +619,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -629,6 +628,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down
6 changes: 3 additions & 3 deletions posthog/api/test/__snapshots__/test_insight.ambr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -822,6 +821,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down Expand Up @@ -1235,7 +1235,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -1245,6 +1244,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down Expand Up @@ -1703,7 +1703,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -1713,6 +1712,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down
8 changes: 4 additions & 4 deletions posthog/api/test/__snapshots__/test_organization_feature_flag.ambr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -269,6 +268,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down Expand Up @@ -438,7 +438,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -448,6 +447,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down Expand Up @@ -613,7 +613,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -623,6 +622,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down Expand Up @@ -783,7 +783,6 @@
"posthog_dashboarditem"."deleted",
"posthog_dashboarditem"."saved",
"posthog_dashboarditem"."created_at",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."refreshing",
"posthog_dashboarditem"."created_by_id",
"posthog_dashboarditem"."is_sample",
Expand All @@ -793,6 +792,7 @@
"posthog_dashboarditem"."last_modified_at",
"posthog_dashboarditem"."last_modified_by_id",
"posthog_dashboarditem"."dashboard_id",
"posthog_dashboarditem"."last_refresh",
"posthog_dashboarditem"."layouts",
"posthog_dashboarditem"."color",
"posthog_dashboarditem"."dive_dashboard_id",
Expand Down
Loading
Loading