Skip to content

Conversation

@fredrikekelund
Copy link
Contributor

We currently run the CLI commands on before_wp_load. There's a brief discussion in #4 (comment) about why we do that, which boils down to ensuring these commands work even when the WP installation is broken. However, messing with the normal WP load order like we do here (we "manually" load the sqlite-database-integration plugin) can be problematic. For example, it caused the following bug in Studio: https://github.com/Automattic/dotcom-forge/issues/10054

I need to do some more testing before we can determine if we should land this change or not, but I've confirmed that this PR fixes https://github.com/Automattic/dotcom-forge/issues/10054.

Any input from @sejas and @wojtekn is very welcome! @sejas and I already paired over this, so you're already very familiar with the problem 😊

@fredrikekelund fredrikekelund self-assigned this Dec 17, 2024
@fredrikekelund
Copy link
Contributor Author

One thing that probably breaks with this change is exporting a site with broken code (like a broken plugin, for example). To mitigate this, we could consider passing the --skip-plugins and --skip-themes options to WP-CLI. mu-plugins are apparently still loaded when passing those flags.

Jeroen mentioned that:

If you try to import into a corrupt/empty/incomplete database, the command will fail if you load wp first. Same goes for export, it should work regardless of the state of the Wordpress installation

I don't entirely understand why this is the case… SQL files can only be imported into existing websites. Those files don't need to contain an entire database; we simply execute the SQL statements in the file.

As for export, I also don't understand what scenario we're talking about when we say "corrupt/empty/incomplete database". I understand how broken code could be problematic, but I already suggested a potential solution to that previously…

@Automattic/yolo, I welcome your input

@sejas
Copy link
Member

sejas commented Dec 17, 2024

Using * @when after_wp_load was a TIL for me.
We also explored the possibility of loading only the SQLite mu-plugin and not the whole WordPress, but the mu-plugin needs functions such as add_action. Loading the polyfills file was another possible solution, but I think loading the whole WordPress is a more robust approach.

The changes in this PR makes sense to me and I like Fredrik's suggestion of adding --skip-plugins --skip-themes 👍 which will reduce to "zero" the plugins incompatibility with WASM.

Thanks for the changes @fredrikekelund !,
Let's give it a test with different new and old sites to see if we spot any unexpected behaviour.

@wojtekn
Copy link
Contributor

wojtekn commented Dec 17, 2024

@sejas @fredrikekelund, what happens when you try to import database dump using wp sqlite import to a site that has a database corrupted, e.g., after dropping wp_options?

@fredrikekelund
Copy link
Contributor Author

Closing this per our discussion in Slack p1734445093481979/1734431651.518169-slack-C04GESRBWKW

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