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.

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

feat(cdp): Added legacy plugins worker #27835

Merged
merged 48 commits into from
Jan 28, 2025
Merged

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Jan 23, 2025

Problem

We want to fast migrate off of plugins to HogFunctions and one way we can do this is by just inlining the plugins and using HogFunctions as the base layer for the actual work

Changes

  • Adds the plugin worker which simulates the plugin server

TODO

  • Detect these "plugin" destinations and queue to cyclotron as "plugin"
  • Add migration command (will test properly and have follow up work for sure
  • Collect logs
  • Fix for customer.io person checking (we could just use persons instead and remove the storage)

👉 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?

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

github-actions bot commented Jan 27, 2025

Size Change: 0 B

Total Size: 1.16 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.16 MB

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@benjackwhite benjackwhite marked this pull request as ready for review January 27, 2025 17:24
posthog/cdp/migrations.py Fixed Show fixed Hide fixed
posthog/cdp/migrations.py Fixed Show fixed Hide fixed
@benjackwhite benjackwhite requested a review from a team January 28, 2025 08:18
Copy link
Contributor

@meikelmosby meikelmosby left a comment

Choose a reason for hiding this comment

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

couple of nits but overall lgtm

@benjackwhite benjackwhite requested a review from pl January 28, 2025 09:34
Copy link
Contributor

@pl pl left a comment

Choose a reason for hiding this comment

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

I reviewed the worker part, but skipped the actual plugin code and migrations, as I'm still out of depth on that.

Changes to the worker LGTM.

@benjackwhite benjackwhite enabled auto-merge (squash) January 28, 2025 11:24
@benjackwhite benjackwhite merged commit cb81e9d into master Jan 28, 2025
99 checks passed
@benjackwhite benjackwhite deleted the feat/cyclotron-plugins branch January 28, 2025 11:51
Copy link

sentry-io bot commented Jan 28, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ValidationError: {"inputs":{"googleCloudKeyJson":"Value must be a string."}} posthog.cdp.migrations in migrate_legacy_plugins View Issue
  • ‼️ SyntaxError: '(' was never closed (migrations.py, line 17) posthog.management.commands.migrate_plugins_to_... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants