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

Enable plugins for everyone #3557

Merged
merged 18 commits into from
Apr 7, 2021
Merged

Enable plugins for everyone #3557

merged 18 commits into from
Apr 7, 2021

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Mar 2, 2021

Changes

This deprecates Team.plugins_opt_in. Plugins will be enabled for everyone on self-hosted and enabled or disabled based on other variables on Cloud (pending #3328).

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl temporarily deployed to posthog-remove-plugins--wj9euz March 2, 2021 17:49 Inactive
@weyert
Copy link
Contributor

weyert commented Mar 3, 2021

So exciting the deployment looking promising :D

@Twixes Twixes marked this pull request as ready for review March 4, 2021 08:14
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 11:58 Inactive
@mariusandra
Copy link
Collaborator

Are we sure we already want to remove "BETA"? Still some things to fix in the plugin system before I'm ready to declare that...

@Twixes
Copy link
Member Author

Twixes commented Mar 4, 2021

Hm, maybe not necessarily in this PR… Though the goal for 1.23 is to indeed put plugins out of beta. Is there something you'd consider a blocker that is not included in our release priorities as they stand?

@mariusandra
Copy link
Collaborator

Once the issues in the release are done (mainly lazy vms and access control... and maybe logs?), we're out of beta (and maybe some other things from this list.

The problem is that some deployments/users deploy from master who will be out of beta, even though we're still beta :). Then again, it's just a label so feel free to remove.

@Twixes
Copy link
Member Author

Twixes commented Mar 4, 2021

Yeah, we'll merge this after the solutions for things you mentioned.

@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 15:07 Inactive
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 15:30 Inactive
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 15:37 Inactive
@Twixes
Copy link
Member Author

Twixes commented Mar 4, 2021

I added two shiny things to mark the end of beta for users:

  • A blip shown on the Plugins navigation item. Hidden on the device after user views the Plugins page once.
    blip

  • A banner shown on the Plugins page. Hidden on the device after user clicks "X" to close.
    banner

I'm advocating for a marketing-type blog post on Plugins, explaining rationale for making this a core feature, its utility, how easy it is to write your own plugin – something of that sort.

@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 15:42 Inactive
@Twixes Twixes changed the title Enable plugins for everyone, leaving opt-in phase Enable plugins for everyone, leaving beta phase Mar 4, 2021
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 19:14 Inactive
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 21:41 Inactive
@Twixes Twixes mentioned this pull request Mar 4, 2021
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 4, 2021 23:53 Inactive
@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 5, 2021 00:13 Inactive
@Twixes
Copy link
Member Author

Twixes commented Mar 5, 2021

Opted for a crossdevice solution replacing local posthog.persistence with a new field on the User model: flags - a schemaless dict saved as JSON, for keeping track of such small things for which whole new schema fields would be overkill, but which nevertheless are much better saved on the User instance.

@Twixes Twixes temporarily deployed to posthog-remove-plugins--wj9euz March 5, 2021 14:26 Inactive
@avorio
Copy link

avorio commented Mar 5, 2021

@Twixes @mariusandra Thanks for the fabulous work you folks are doing here.

I've got a question: my plugin server is not working, as described in this issue. It's the only item showing a red "NO" in the system status page. However, I have also noticed that my events stopped appearing in PostHog after I upgraded to the latest version of PostHog, 1.22.0. Am I losing data since that upgrade? According to the release notes, the plugin server was going to be mandatory in the next release, which I understood to be 1.23.0.

How can I get out of this, please? Many-many thanks.

@Twixes Twixes requested a review from macobo March 18, 2021 02:01
Copy link
Contributor

@macobo macobo left a comment

Choose a reason for hiding this comment

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

Added some notes.

The weakest part here imo is the "highlight" or tutorials approach. Seems like something we'll likely solve elsehow at some point. Did you intend for this to be a more permanent solution?

frontend/src/layout/navigation/MainNavigation.tsx Outdated Show resolved Hide resolved
frontend/src/scenes/plugins/Plugins.tsx Outdated Show resolved Hide resolved
frontend/src/scenes/plugins/Plugins.tsx Outdated Show resolved Hide resolved
frontend/src/layout/navigation/MainNavigation.tsx Outdated Show resolved Hide resolved
posthog/api/user.py Outdated Show resolved Hide resolved
@mariusandra
Copy link
Collaborator

I'd scale this down... and just silently remove the plugins_opt_in code without any ceremony. Introducing a new field flags on users to store a toggle and all this alerting seems like over-engineering to me. Removing the danger zone and the opt-in screen is definitely something we want to do, but I'm not convinced on the other parts.

I think that plugins are also not yet ready to go out of BETA. I'd keep the label around still for some time to slightly deflect blame in case something is not working well (e.g. we have no retries, etc).

@Twixes Twixes mentioned this pull request Mar 26, 2021
5 tasks
@Twixes Twixes requested a review from mariusandra April 1, 2021 20:50
@Twixes
Copy link
Member Author

Twixes commented Apr 1, 2021

Scaled down @mariusandra

@Twixes Twixes changed the title Enable plugins for everyone, leaving beta phase Enable plugins for everyone Apr 2, 2021
Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

LGTM

@Twixes Twixes merged commit 61feb50 into master Apr 7, 2021
@Twixes Twixes deleted the remove-plugins-opt-out branch April 7, 2021 14:43
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.

6 participants