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

Ensure the plugin works smoothly when activating it on a site with Performance Lab SQLite module #33

Closed
felixarntz opened this issue Apr 25, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@felixarntz
Copy link
Member

We need to test that the plugin works in combination with the Performance Lab plugin's SQLite module. Specifically, if a site already has the Performance Lab SQLite module active, we need to ensure that activating the standalone plugin at that point works correctly.

For example:

  • It should not throw a fatal error due to e.g. duplicate functions.
  • It should continue to operate smoothly even if the Performance Lab's SQLite module db.php drop-in is currently placed.

The work here potentially involves making changes to both the PL module and the standalone plugin. Since the standalone plugin is going to be the source of truth going forward (see WordPress/performance#661 (comment)), we should try to limit the changes needed in the PL module though.

@felixarntz felixarntz added the enhancement New feature or request label Apr 25, 2023
@felixarntz
Copy link
Member Author

cc @aristath @joemcgill

@aristath
Copy link
Member

aristath commented May 8, 2023

Specifically, if a site already has the Performance Lab SQLite module active, we need to ensure that activating the standalone plugin at that point works correctly.

Confirmed. I had to push a commit to allow upgrading from the PL module to the standalone plugin, but it works fine now.
The problem was that the db.php file does a require_once to load the files from the plugin, so the file needs to be updated when we switch plugins, so that the new files get required instead of the old ones.
When the standalone plugin gets activated, it now checks the db.php file, and if we detect that the PERFLAB_SQLITE_DB_DROPIN_VERSION constant is defined (in which case the file comes from PL, the user sees a warning on their admin screen in the dashboard, and they can click a button to update the file and switch to the snandalone plugin.

It should not throw a fatal error due to e.g. duplicate functions.

Confirmed.

The work here potentially involves making changes to both the PL module and the standalone plugin.

What changes would we like to make to the PL plugin? Should we remove the SQLite implementation from there, and instead add an admin notice prompting users to install the standalone plugin?

@felixarntz
Copy link
Member Author

Thanks @aristath for the update!

I made one comment 6e5b9e8#r112417524 on the commit for something that would be good to fix, but other than that it looks great to me.

When are you planning to release the plugin version that includes this logic?

What changes would we like to make to the PL plugin? Should we remove the SQLite implementation from there, and instead add an admin notice prompting users to install the standalone plugin?

Yes, but we also need to consider a smooth migration path. For example, I think we need to first launch a version with the admin notice, and only then launch a version where the module is actually removed, just to give people a chance to update before we would potentially break their site by removing the module.

@aristath
Copy link
Member

aristath commented May 9, 2023

When are you planning to release the plugin version that includes this logic?

Already done 🎉

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

No branches or pull requests

2 participants