-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
chore(data-warehouse): add data warehouse type to experiment trend goal #21053
Conversation
Size Change: 0 B Total Size: 824 kB ℹ️ View Unchanged
|
@@ -131,6 +135,15 @@ export function ExperimentInsightCreator({ insightProps }: { insightProps: Insig | |||
TaxonomicFilterGroupType.Elements, | |||
TaxonomicFilterGroupType.HogQLExpression, | |||
]} | |||
actionsTaxonomicGroupTypes={ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the backend feeds into the old trends query runner class, will that handle dataWarehouse actions? Basically this line: https://github.com/PostHog/posthog/blob/master/ee/clickhouse/queries/experiments/trend_experiment_result.py#L225 unsure if this automagically supports these new types.
Not sure if you've tried running an experiment locally yet. We'll also add a breakdown by feature flag to the metric, so I imagine this table needs to have the flag properties as well? Unless its a funnel with the first event coming from non-data warehouse (which will automatically populate it) - but I assume that still requires a personID column in the warehouse to match the right persons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh hmm, if the backend depends on old trends query runner, this won't work yet
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
Problem
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?