feat(experiments): cache experiment results #14742
Conversation
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
| @@ -525,7 +525,7 @@ export const experimentLogic = kea<experimentLogicType>([ | |||
| const response = await api.get( | |||
| `api/projects/${values.currentTeamId}/experiments/${values.experimentId}/results` | |||
| ) | |||
| return { ...response, fakeInsightId: Math.random().toString(36).substring(2, 15) } | |||
| return { ...response.result, fakeInsightId: Math.random().toString(36).substring(2, 15) } | |||
There was a problem hiding this comment.
presumably the errors in the visual regression tests come from this?
| return { ...response.result, fakeInsightId: Math.random().toString(36).substring(2, 15) } | |
| return { ...(response?.result || {}), fakeInsightId: Math.random().toString(36).substring(2, 15) } |
but I didn't run the code so 🙈
There was a problem hiding this comment.
so I thought about this as well, but {} will throw an error down the line.
I didn't improve things here, but didn't make them worse either, because if the result that now comes inside response.result wasn't at the top-level before, we'd also throw
There was a problem hiding this comment.
Hmm, this will also throw right on deploy, but that shouldn't be too long.
There was a problem hiding this comment.
I'll consider how to best handle things
frontend/src/scenes/insights/EditorFilters/samplingFilterLogic.ts
Outdated
Show resolved
Hide resolved
frontend/src/scenes/insights/EditorFilters/samplingFilterLogic.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Paul D'Ambra <paul@posthog.com>
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
|
ready for a final look @neilkakkar @pauldambra |
There was a problem hiding this comment.
lol this is clearly not good - looking
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
* master: (101 commits) feat: feedback tab - improve the UI and switch to HogQL (#14777) feat(experiments): cache experiment results (#14742) fix(empty-state): properly fix query timeout state (#14793) chore(feature-flag): release json payload flag (#14775) feat(hogql): make sure joins work with properties (#14790) test(frontend): Tune visual regression test failure threshold (#14766) feat(hogql): count distinct and count star (#14786) feat(perf): only load experiment results once (#14772) feat: allow new insight type choice from sidebar (#14759) fix(persons): hide persons in the future (#14308) feat: allow query cards in saved insights grid (#14784) fix(lemon-ui): Align padding for small/large buttons with icons (#14750) chore(deps): Upgrade TypeScript from 4.8 to 4.9 in the frontend (#14609) feat: rename insight query tab to json tab (#14781) feat: use same top heading for insight card and exported insight (#14780) feat: define new insight urls in one place (#14778) fix(hogql): person properties in a funnel breakdown (#14765) fix: hide query based insights in recently viewed list (#14774) feat: improve query summaries (#14768) feat(hogql): add sampling support (#14733) ...
Adding some initial caching to experiment results.
This isn't in its ideal state, but I opted for getting the ball rolling and iterating. Currently caching experiment results for 30min only with some bespoke logic meant for them only. This should at least prevent us from recalculating on refreshes and changes during a single session.
Essentially got the essence of the
@cached_functionlogic and implemented it for experiments. Unsure if it's the best approach. Also we seem to not test@cached_functionanywhere? Although I could add some tests hereAlso includes a fix for the fact that
setFiltersisn't called when setting an experiment goal which was leaving the sampling toggle off even though the insight was sampled.