Skip to content

Commit

Permalink
test(insights): Run all tests with filters-based insights using Hog…
Browse files Browse the repository at this point in the history
…QL (#21861)

* ci: Run core backend tests with both HogQL and legacy insights

* Double the number of tests

* Fix env var setting

* Add some HOGQL_INSIGHTS_OVERRIDE overrides in tests

* Mark `Insight.last_refresh` as deprecated

* Fix bad merge

* Update query snapshots

* Update query snapshots

* Update test_team.py

* Actually remove legacy backend from matrix

* Update test_fetch_from_cache.py

* Use `update_cached_state` in `calculate_for_query_based_insight()`

* Clean up CI changes and a comment

* Fix `update_cached_state` typing

* Update test_fetch_from_cache.py

* Update test_insight_cache.py

* Update test_insight_cache.py

* Clarify `generate_insight_cache_key` as legacy function

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Twixes and github-actions[bot] authored May 23, 2024
1 parent 739123b commit b1cb745
Show file tree
Hide file tree
Showing 17 changed files with 84 additions and 53 deletions.
1 change: 1 addition & 0 deletions .github/actions/run-backend-tests/action.yml
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
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

# 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
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,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 @@ -348,7 +351,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
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
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
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

0 comments on commit b1cb745

Please sign in to comment.