Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: yeet CH recordings ingestion #17572

Merged
merged 48 commits into from
Oct 11, 2023
Merged

chore: yeet CH recordings ingestion #17572

merged 48 commits into from
Oct 11, 2023

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Sep 21, 2023 •
edited
Loading

Removing ClickHouse based recordings

One big yeet for a man, a great yeet for humanity

benjackwhite
benjackwhite requested changes Sep 21, 2023
View reviewed changes
posthog/api/capture.py Outdated Show resolved Hide resolved
@pauldambra pauldambra marked this pull request as draft September 21, 2023 12:15
@pauldambra pauldambra mentioned this pull request Sep 21, 2023
Closed
@pauldambra
Copy link
Member Author

OK, I can definitely ingest and playback recordings at commit b2ce960 of this PR

pauldambra
pauldambra commented Sep 22, 2023
View reviewed changes
plugin-server/functional_tests/session-recordings.test.ts Outdated
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these haven't been testing the new flow anyway, and I couldn't figure out what the workflow is to fix them so skip for now

@pauldambra pauldambra changed the title feat: step 1 - remove recordings-ingestion capability from plugin server yeet CH ingestion Sep 22, 2023
@pauldambra pauldambra changed the title yeet CH ingestion chore: yeet CH recordings ingestion Sep 22, 2023
@pauldambra pauldambra marked this pull request as ready for review September 27, 2023 13:36
benjackwhite
benjackwhite requested changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of things but this is generally dope. There's probably all sorts of small things left over but this work is the bulk that enables us to keep cleaning.

Love it!

frontend/src/types.ts Outdated Show resolved Hide resolved
posthog/session_recordings/models/session_recording.py Show resolved Hide resolved
posthog/settings/ingestion.py Outdated Show resolved Hide resolved
@benjackwhite benjackwhite self-requested a review September 28, 2023 08:57
xvello
xvello approved these changes Oct 4, 2023
View reviewed changes
Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@pauldambra pauldambra force-pushed the feat/remove-legacy-ingestion-capability-from-plugin-server branch 2 times, most recently from 66a2e9b to 7f54b72 Compare October 11, 2023 10:21
benjackwhite
benjackwhite approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@benjackwhite benjackwhite left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its good 😬
Ran it locally with no issues so 🖕🖕🖕🖕🖕 🤞🤞🤞🤞🤞 (used the wrong emoji and only noticed afterwards - leaving in cos its funny :D )

@pauldambra pauldambra merged commit 31c1cdf into master Oct 11, 2023
75 checks passed
@pauldambra pauldambra deleted the feat/remove-legacy-ingestion-capability-from-plugin-server branch October 11, 2023 13:23
pauldambra added a commit that referenced this pull request Oct 11, 2023
@pauldambra
Revert "chore: yeet CH recordings ingestion (#17572)"
a8e311b 
This reverts commit 31c1cdf.
@neilkakkar
Copy link
Collaborator

I got emailed about your comment (without the edited changes), so yes I had to come back here to see what's going on 😂

daibhin pushed a commit that referenced this pull request Oct 23, 2023
@pauldambra @daibhin
chore: yeet CH recordings ingestion (#17572)
06cb45b 
Removing ClickHouse based recordings

One big yeet for a man, a great yeet for humanity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xvello xvello xvello approved these changes

@benjackwhite benjackwhite benjackwhite approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pauldambra @posthog-bot @neilkakkar @benjackwhite @xvello