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: populate plugin capabilities on install and edit #19188

Merged
merged 3 commits into from
Jan 3, 2024

Conversation

tiina303
Copy link
Contributor

@tiina303 tiina303 commented Dec 7, 2023

Problem

When we install or edit source plugins we don't have their capabilities set. This was fine in the old UI, where everything was together, but in the new pipeline UI we separate transformation plugins from destinations, so in order of the right plugins to show up we'll want to have the capabilities populated.
Using the scheduler pod as we don't have that many tasks there.
Note that normally the capabilities get populated on first usage of the app.

context: https://posthog.slack.com/archives/C0374DA782U/p1701960067687139

Changes

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

Created some source apps and edited their code and verified that capabilities were updated correctly.

@tiina303 tiina303 requested review from xvello and a team December 7, 2023 17:42
@bretthoerner bretthoerner force-pushed the populate-plugin-capabilities branch from 0be0e0d to 977cb0c Compare December 11, 2023 15:38
@bretthoerner
Copy link
Contributor

I clicked the Rebase button to pull in the test fix from Friday.

Comment on lines 71 to 75
// `posthog_plugin` columns have to be listed individually, as we want to exclude a few columns
// and Postgres syntax unfortunately doesn't have a column exclusion feature. The excluded columns are:
// - archive - this is a potentially large blob, only extracted in Django as a plugin server optimization
// - latest_tag - not used in this service
// - latest_tag_checked_at - not used in this service
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// `posthog_plugin` columns have to be listed individually, as we want to exclude a few columns
// and Postgres syntax unfortunately doesn't have a column exclusion feature. The excluded columns are:
// - archive - this is a potentially large blob, only extracted in Django as a plugin server optimization
// - latest_tag - not used in this service
// - latest_tag_checked_at - not used in this service

columns are listed individually because it's a good practice when you're developing for a bigger scale than a laptop, we can drop the long comment

@@ -393,6 +393,12 @@ export async function startPluginsServer(
'reset-available-features-cache': async (message) => {
await piscina?.broadcastTask({ task: 'resetAvailableFeaturesCache', args: JSON.parse(message) })
},
'populate-plugin-capabilities': async (message) => {
// We need this to be done in only once
if (hub?.capabilities.pluginScheduledTasks && piscina) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd rename the transpileFrontendApps capability into appManagementSingleton and reuse it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was so confused ... the transpileFrontedApps isn't used anymore (Marius moved that code out), so definitely calls for renaming or removal

@posthog-bot
Copy link
Contributor

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 stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@tiina303 tiina303 reopened this Jan 2, 2024
@tiina303 tiina303 removed the stale label Jan 2, 2024
@tiina303 tiina303 force-pushed the populate-plugin-capabilities branch from 977cb0c to 0e920c5 Compare January 3, 2024 13:11
@tiina303 tiina303 merged commit ed1611a into master Jan 3, 2024
72 of 73 checks passed
@tiina303 tiina303 deleted the populate-plugin-capabilities branch January 3, 2024 14:26
fuziontech added a commit that referenced this pull request Jan 4, 2024
* master: (94 commits)
  feat: Add cta on billing for >20k annual spend customers (#19508)
  refactor(temporal/squash): Support flat person override table in squash workflow (#19347)
  fix(surveys): remove link in user interview template (#19584)
  feat: populate plugin capabilities on install and edit (#19188)
  chore: Make plugin-server ignore deleted plugin configs (#18444)
  chore: Add Flutter feature flags snippets (#19563)
  fix(bi): fixed some of the query duplications (#19573)
  fix: assume typeless series nodes are of type events node (#19550)
  chore(deps): bump chromaui/action from 1 to 10 (#19560)
  fix(trends): fix breakdowns persons label (#19534)
  fix(trends): fix breakdown legend (#19533)
  feat: incremental updates for mobile transformer (#19567)
  chore(deps): bump peter-evans/find-comment from 1 to 2 (#19559)
  chore(data-warehouse): cleanup unused celery code and extend time (#19568)
  Revert "feat(data-warehouse): hubspot integration" (#19569)
  feat(data-warehouse): hubspot integration (#19529)
  feat: Feature gate session replay controls using available_product_features (#19401)
  fix: Padding on bullet lists (#19565)
  chore: Post 3000 LemonButtton cleanup (#19540)
  chore(deps): bump aws-actions/amazon-ecr-login from 1 to 2 (#19558)
  ...
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