Skip to content

Commit

Permalink
Merge branch 'master' into dn-chore/remove-ingestion-experiment
Browse files Browse the repository at this point in the history
  • Loading branch information
daibhin committed Apr 16, 2024
2 parents 36853a6 + 5f5d6a9 commit 118c4e1
Show file tree
Hide file tree
Showing 21 changed files with 168 additions and 263 deletions.
4 changes: 2 additions & 2 deletions bin/check_temporal_up
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ while true; do
fi

if [ $SECONDS -ge $TIMEOUT ]; then
echo "Timed out waiting for Temporal to be ready"
exit 1
echo "Timed out ($TIMEOUT sec) waiting for Temporal to be ready. Crossing fingers and trying to run tests anyway."
exit 0
fi

echo "Waiting for Temporal to be ready"
Expand Down
1 change: 0 additions & 1 deletion frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ export enum NodeKind {
SavedInsightNode = 'SavedInsightNode',
InsightVizNode = 'InsightVizNode',

// New queries, not yet implemented
TrendsQuery = 'TrendsQuery',
FunnelsQuery = 'FunnelsQuery',
RetentionQuery = 'RetentionQuery',
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/experiments/ExperimentView/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ export function ActionBanner(): JSX.Element {
// Win probability only slightly over 0.9 and the recommended sample/time just met -> proceed with caution
if (
experimentInsightType === InsightType.FUNNELS &&
funnelResultsPersonsTotal > recommendedSampleSize + 50 &&
funnelResultsPersonsTotal < recommendedSampleSize + 50 &&
winProbability < 0.93
) {
return (
Expand All @@ -432,7 +432,7 @@ export function ActionBanner(): JSX.Element {

if (
experimentInsightType === InsightType.TRENDS &&
actualRunningTime > recommendedRunningTime + 2 &&
actualRunningTime < recommendedRunningTime + 2 &&
winProbability < 0.93
) {
return (
Expand Down
20 changes: 0 additions & 20 deletions frontend/src/scenes/persons/personsLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,26 +101,6 @@ export const personsLogic = kea<personsLogicType>([
null as PersonType | null,
{
loadPerson: async ({ id }): Promise<PersonType | null> => {
if (values.featureFlags[FEATURE_FLAGS.PERSONS_HOGQL_QUERY]) {
const response = await hogqlQuery(
'select id, groupArray(pdi.distinct_id) as distinct_ids, properties, is_identified, created_at from persons where pdi.distinct_id={distinct_id} group by id, properties, is_identified, created_at',
{ distinct_id: id }
)
const row = response?.results?.[0]
if (row) {
const person: PersonType = {
id: row[0],
uuid: row[0],
distinct_ids: row[1],
properties: JSON.parse(row[2] || '{}'),
is_identified: !!row[3],
created_at: row[4],
}
actions.reportPersonDetailViewed(person)
return person
}
}

const response = await api.persons.list({ distinct_id: id })
const person = response.results[0]
if (person) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ export const sessionPlayerModalLogic = kea<sessionPlayerModalLogicType>([
],
}),
actionToUrl(({ values }) => {
const buildURL = (
replace: boolean
): [
const buildURL = (): [
string,
Record<string, any>,
Record<string, any>,
Expand All @@ -71,12 +69,12 @@ export const sessionPlayerModalLogic = kea<sessionPlayerModalLogicType>([
searchParams.timestamp = values.initialTimestamp
}

return [router.values.location.pathname, searchParams, hashParams, { replace }]
return [router.values.location.pathname, searchParams, hashParams, { replace: true }]
}

return {
openSessionPlayer: () => buildURL(false),
closeSessionPlayer: () => buildURL(false),
openSessionPlayer: () => buildURL(),
closeSessionPlayer: () => buildURL(),
}
}),
urlToAction(({ actions, values }) => {
Expand Down
8 changes: 0 additions & 8 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Argument
posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Dict entry 0 has incompatible type "str": "PathsFilter"; expected "str": "TrendsFilter" [dict-item]
posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Dict entry 0 has incompatible type "str": "LifecycleFilter"; expected "str": "TrendsFilter" [dict-item]
posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Dict entry 0 has incompatible type "str": "StickinessFilter"; expected "str": "TrendsFilter" [dict-item]
posthog/hogql_queries/legacy_compatibility/feature_flag.py:0: error: Item "AnonymousUser" of "User | AnonymousUser" has no attribute "email" [union-attr]
posthog/api/utils.py:0: error: Incompatible types in assignment (expression has type "type[EventDefinition]", variable has type "type[EnterpriseEventDefinition]") [assignment]
posthog/api/utils.py:0: error: Argument 1 to "UUID" has incompatible type "int | str"; expected "str | None" [arg-type]
ee/billing/quota_limiting.py:0: error: List comprehension has incompatible type List[int]; expected List[str] [misc]
Expand Down Expand Up @@ -256,7 +255,6 @@ posthog/hogql/transforms/in_cohort.py:0: error: Incompatible default for argumen
posthog/hogql/transforms/in_cohort.py:0: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
posthog/hogql/transforms/in_cohort.py:0: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
posthog/hogql/transforms/in_cohort.py:0: error: Argument "is_static" to "_add_join_for_cohort" of "InCohortResolver" has incompatible type "bool | None"; expected "bool" [arg-type]
posthog/hogql/transforms/in_cohort.py:0: error: Incompatible types in assignment (expression has type "ValuesQuerySet[Cohort, tuple[int, bool | None]]", variable has type "ValuesQuerySet[Cohort, tuple[int, bool | None, str | None]]") [assignment]
posthog/hogql/transforms/in_cohort.py:0: error: Argument "is_static" to "_add_join_for_cohort" of "InCohortResolver" has incompatible type "bool | None"; expected "bool" [arg-type]
posthog/hogql/transforms/in_cohort.py:0: error: Argument "table" to "JoinExpr" has incompatible type "Expr"; expected "SelectQuery | SelectUnionQuery | Field | None" [arg-type]
posthog/hogql/transforms/in_cohort.py:0: error: List item 0 has incompatible type "SelectQueryType | None"; expected "SelectQueryType" [list-item]
Expand Down Expand Up @@ -351,7 +349,6 @@ posthog/queries/breakdown_props.py:0: error: Argument 1 to "translate_hogql" has
posthog/queries/funnels/base.py:0: error: "HogQLContext" has no attribute "person_on_events_mode" [attr-defined]
posthog/queries/funnels/base.py:0: error: Argument 1 to "translate_hogql" has incompatible type "str | int"; expected "str" [arg-type]
ee/clickhouse/queries/funnels/funnel_correlation.py:0: error: Statement is unreachable [unreachable]
posthog/caching/calculate_results.py:0: error: Argument 3 to "process_query" has incompatible type "bool"; expected "LimitContext | None" [arg-type]
posthog/api/person.py:0: error: Argument 1 to "loads" has incompatible type "str | None"; expected "str | bytes | bytearray" [arg-type]
posthog/api/person.py:0: error: Argument "user" to "log_activity" has incompatible type "User | AnonymousUser"; expected "User | None" [arg-type]
posthog/api/person.py:0: error: Argument "user" to "log_activity" has incompatible type "User | AnonymousUser"; expected "User | None" [arg-type]
Expand Down Expand Up @@ -391,11 +388,6 @@ posthog/hogql_queries/insights/lifecycle_query_runner.py:0: note: Consider using
posthog/hogql_queries/insights/lifecycle_query_runner.py:0: error: Argument 1 to "sorted" has incompatible type "list[Any] | None"; expected "Iterable[Any]" [arg-type]
posthog/hogql_queries/insights/lifecycle_query_runner.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "select_from" [union-attr]
posthog/hogql_queries/insights/lifecycle_query_runner.py:0: error: Item "None" of "JoinExpr | Any | None" has no attribute "sample" [union-attr]
posthog/hogql_queries/legacy_compatibility/process_insight.py:0: error: Incompatible types in assignment (expression has type "PathFilter", variable has type "RetentionFilter") [assignment]
posthog/hogql_queries/legacy_compatibility/process_insight.py:0: error: Incompatible types in assignment (expression has type "StickinessFilter", variable has type "RetentionFilter") [assignment]
posthog/hogql_queries/legacy_compatibility/process_insight.py:0: error: Incompatible types in assignment (expression has type "Filter", variable has type "RetentionFilter") [assignment]
posthog/api/insight.py:0: error: Argument 1 to "is_insight_with_hogql_support" has incompatible type "Insight | DashboardTile"; expected "Insight" [arg-type]
posthog/api/insight.py:0: error: Argument 1 to "process_insight" has incompatible type "Insight | DashboardTile"; expected "Insight" [arg-type]
posthog/api/dashboards/dashboard.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/api/feature_flag.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/api/feature_flag.py:0: error: Item "Sequence[Any]" of "Any | Sequence[Any] | None" has no attribute "filters" [union-attr]
Expand Down
45 changes: 30 additions & 15 deletions posthog/api/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@
from posthog.api.tagged_item import TaggedItemSerializerMixin, TaggedItemViewSetMixin
from posthog.api.utils import format_paginated_url
from posthog.auth import SharingAccessTokenAuthentication
from posthog.caching.fetch_from_cache import (
InsightResult,
fetch_cached_insight_result,
synchronously_update_cache,
)
from posthog.caching.fetch_from_cache import InsightResult, fetch_cached_insight_result, synchronously_update_cache
from posthog.caching.insights_api import should_refresh_insight
from posthog.constants import (
BREAKDOWN_VALUES_LIMIT,
Expand All @@ -59,7 +55,7 @@
from posthog.hogql.timings import HogQLTimings
from posthog.hogql_queries.apply_dashboard_filters import DATA_TABLE_LIKE_NODE_KINDS
from posthog.hogql_queries.legacy_compatibility.feature_flag import hogql_insights_enabled
from posthog.hogql_queries.legacy_compatibility.process_insight import is_insight_with_hogql_support, process_insight
from posthog.hogql_queries.legacy_compatibility.filter_to_query import filter_to_query
from posthog.kafka_client.topics import KAFKA_METRICS_TIME_TO_SEE_DATA
from posthog.models import DashboardTile, Filter, Insight, User
from posthog.models.activity_logging.activity_log import (
Expand Down Expand Up @@ -521,14 +517,34 @@ def to_representation(self, instance: Insight):

@lru_cache(maxsize=1)
def insight_result(self, insight: Insight) -> InsightResult:
dashboard = self.context.get("dashboard", None)
dashboard_tile = self.dashboard_tile_from_context(insight, dashboard)
target = insight if dashboard is None else dashboard_tile
from posthog.caching.calculate_results import calculate_for_query_based_insight

if insight.query:
try:
return calculate_for_query_based_insight(
insight, refresh_requested=refresh_requested_by_client(self.context["request"])
)
except ExposedHogQLError as e:
raise ValidationError(str(e))

if hogql_insights_enabled(self.context.get("request", None).user) and is_insight_with_hogql_support(
target or insight
if not self.context["request"].user.is_anonymous and hogql_insights_enabled(
self.context["request"].user, insight.filters.get("insight", schema.InsightType.TRENDS)
):
return process_insight(target or insight, insight.team)
# TRICKY: As running `filters`-based insights on the HogQL-based engine is a transitional mechanism,
# we fake the insight being properly `query`-based.
# To prevent the lie from accidentally being saved to Postgres, we roll it back in the `finally` branch.
insight.query = filter_to_query(insight.filters).model_dump()
try:
return calculate_for_query_based_insight(
insight, refresh_requested=refresh_requested_by_client(self.context["request"])
)
except ExposedHogQLError as e:
raise ValidationError(str(e))
finally:
insight.query = None

dashboard = self.context.get("dashboard", None)
dashboard_tile = self.dashboard_tile_from_context(insight, dashboard)

is_shared = self.context.get("is_shared", False)
refresh_insight_now, refresh_frequency = should_refresh_insight(
Expand All @@ -539,10 +555,9 @@ def insight_result(self, insight: Insight) -> InsightResult:
)
if refresh_insight_now:
INSIGHT_REFRESH_INITIATED_COUNTER.labels(is_shared=is_shared).inc()
return synchronously_update_cache(insight, dashboard, refresh_frequency)
return synchronously_update_cache(insight, dashboard, refresh_frequency=refresh_frequency)

# :TODO: Clear up if tile can be null or not
return fetch_cached_insight_result(target or insight, refresh_frequency)
return fetch_cached_insight_result(dashboard_tile or insight, refresh_frequency)

@lru_cache(maxsize=1) # each serializer instance should only deal with one insight/tile combo
def dashboard_tile_from_context(self, insight: Insight, dashboard: Optional[Dashboard]) -> Optional[DashboardTile]:
Expand Down
2 changes: 1 addition & 1 deletion posthog/api/property_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def with_event_property_filter(
event_name_join_filter=event_name_join_filter,
event_name_filter=event_name_filter,
event_property_join_type="INNER JOIN" if filter_by_event_names else "LEFT JOIN",
params={**self.params, "event_names": event_names or []},
params={**self.params, "event_names": list(map(str, event_names or []))},
)

def with_search(self, search_query: str, search_kwargs: Dict) -> "QueryContext":
Expand Down
2 changes: 2 additions & 0 deletions posthog/api/services/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
def process_query(
team: Team,
query_json: dict,
*,
limit_context: Optional[LimitContext] = None,
refresh_requested: Optional[bool] = False,
) -> dict:
Expand All @@ -75,6 +76,7 @@ def process_query(
def process_query_model(
team: Team,
query: BaseModel, # mypy has problems with unions and isinstance
*,
limit_context: Optional[LimitContext] = None,
refresh_requested: Optional[bool] = False,
) -> dict:
Expand Down
20 changes: 17 additions & 3 deletions posthog/api/test/dashboards/test_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,14 @@ def test_create_from_template_json_cam_provide_query_tile(self) -> None:
"tiles": [
{
"type": "INSIGHT",
"query": {"kind": "a datatable"},
"query": {
"kind": "DataTableNode",
"columns": ["person", "id", "created_at", "person.$delete"],
"source": {
"kind": "EventsQuery",
"select": ["*"],
},
},
"filters": {"date_from": None},
"layouts": {},
}
Expand Down Expand Up @@ -1277,8 +1284,15 @@ def test_create_from_template_json_cam_provide_query_tile(self) -> None:
"name": None,
"next_allowed_client_refresh": None,
"order": None,
"query": {"kind": "a datatable"},
"result": None,
"query": {
"kind": "DataTableNode",
"columns": ["person", "id", "created_at", "person.$delete"],
"source": {
"kind": "EventsQuery",
"select": ["*"],
},
},
"result": [],
"saved": False,
"short_id": ANY,
"tags": [],
Expand Down
17 changes: 7 additions & 10 deletions posthog/api/test/test_insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -1973,7 +1973,7 @@ def test_get_recently_viewed_insights_excludes_query_based_insights_by_default(s
"*",
"event",
"person",
"coalesce(properties.$current_url, properties.$screen_name) # Url / Screen",
"coalesce(properties.$current_url, properties.$screen_name)",
"properties.$lib",
"timestamp",
],
Expand Down Expand Up @@ -2014,7 +2014,7 @@ def test_get_recently_viewed_insights_can_include_query_based_insights(self) ->
"*",
"event",
"person",
"coalesce(properties.$current_url, properties.$screen_name) # Url / Screen",
"coalesce(properties.$current_url, properties.$screen_name)",
"properties.$lib",
"timestamp",
],
Expand Down Expand Up @@ -2953,22 +2953,19 @@ def test_insight_retention_hogql(self) -> None:
def test_insight_with_filters_via_hogql(self) -> None:
filter_dict = {"insight": "LIFECYCLE", "events": [{"id": "$pageview"}]}

Insight.objects.create(
insight = Insight.objects.create(
filters=Filter(data=filter_dict).to_dict(),
team=self.team,
short_id="xyz123",
)

# fresh response
response = self.client.get(f"/api/projects/{self.team.id}/insights/?short_id=xyz123")
response = self.client.get(f"/api/projects/{self.team.id}/insights/{insight.id}/?refresh=true")
self.assertEqual(response.status_code, status.HTTP_200_OK)

self.assertEqual(len(response.json()["results"]), 1)
self.assertEqual(response.json()["results"][0]["result"][0]["data"], [0, 0, 0, 0, 0, 0, 0, 0])
self.assertEqual(response.json()["result"][0]["data"], [0, 0, 0, 0, 0, 0, 0, 0])

# cached response
response = self.client.get(f"/api/projects/{self.team.id}/insights/?short_id=xyz123")
response = self.client.get(f"/api/projects/{self.team.id}/insights/{insight.id}/?refresh=true")
self.assertEqual(response.status_code, status.HTTP_200_OK)

self.assertEqual(len(response.json()["results"]), 1)
self.assertEqual(response.json()["results"][0]["result"][0]["data"], [0, 0, 0, 0, 0, 0, 0, 0])
self.assertEqual(response.json()["result"][0]["data"], [0, 0, 0, 0, 0, 0, 0, 0])
Loading

0 comments on commit 118c4e1

Please sign in to comment.