fix(sessions): Limit the teams for which we ingest sessions v1 events #25766
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Daesgar
approved these changes
Oct 23, 2024
robbie-c
force-pushed
the
feature/limit_sessions_v1_teams
branch
from
9123769 to
a585101
Compare
robbie-c
force-pushed
the
feature/limit_sessions_v1_teams
branch
from
a585101 to
91e848d
Compare
|
Hey @robbie-c! 👋 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We're currently pulling all events into both the sessions v1 and v2 tables, but only a few times are using v1.
Changes
This PR changes this so that only the teams that are actually using v1 will get their events pulled into it.
I also deleted a bunch of tests for the v1 sessions table, as it was hard to get them working with the new restrictions and the effort v reward was not there for keeping them! The core data model tests are still there.
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Updated the tests :) a similar change has already been made on prod anyway, this is just codifying it.