-
Notifications
You must be signed in to change notification settings - Fork 3
Add Wiki/DropDatabase Command #997
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
base: main
Are you sure you want to change the base?
Conversation
app/Console/Commands/Wiki/Delete.php
Outdated
| $wikiDb = $wiki->wikidb; | ||
| $prefix = $wikiDb->prefix; | ||
|
|
||
| // Replaces current mw database connection config with scoped wiki credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work after being run? I'm not certain what this is replacing but I think this was the wikimanager type credentials right?
Doesn't this mean that unless the app is restarted then this connection will no longer have sufficient powers to do work on other Wikis?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this changes this configuration at runtime - which should only affect this very instance of php running this command, not affecting other instances where Laravel runs (try scrambling the config in tinker like this for example)
It serves several purposes:
- scope the db access to only the affected wiki
- use the connection settings usually used to connect to the mw dbs
- allow to run this command on any API pods (as only the queue deployment gets the root(!) credentials for the mw dbs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored to not directly use the mw connection config, but to create a new one by copying and altering it at runtime
app/Console/Commands/Wiki/Delete.php
Outdated
| } | ||
|
|
||
| $mediawikiPdo = $mwConn->getPdo(); | ||
| $statement = $mediawikiPdo->prepare("UPDATE ${prefix}_user SET user_real_name = '', user_email = '', user_password = ''"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO rather than poking in the raw internals of MW it would make more sense to run a MW maintenance script that does this;
I think this better follows the pattern we have been following before of keeping MW writing to MW's DB.
What would you think about that? I could imagine that either being run from the platform API or just as a standalone command run from a devs laptop.
Maybe following the pattern like https://github.com/wmde/wbaas-deploy/blob/937b2d1a7cf16899d53ad8bd9bfa49a57c88e836/k8s/jobs/addPlatformReservedUserToBotGroup.sh
but running https://github.com/wbstack/mediawiki/blob/main/dist/maintenance/deleteUserEmail.php , https://github.com/wbstack/mediawiki/blob/main/dist/maintenance/changePassword.php ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in theory yes
in reality currently i want to make sure we dont mess up the db while satisfying user requests and minimizing unplanned work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored to drop the whole wiki database instead of blanking certain fields
| class DropDatabaseTest extends TestCase { | ||
| use DatabaseTransactions; | ||
|
|
||
| public function testWikiNotFound() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, integration testing this is almost always more hassle than it's worth
| } | ||
|
|
||
| // Creates a temporary DB connection with wiki scoped credentials | ||
| private function getWikiDbConnection(WikiDb $wikiDb): mixed { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love that you managed to make this work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me too - there seems to be quite the difference though between calling config() and app->config and i didn't find out what it is yet (i.e., with config() this didn't work iirc)
|
Converted to Draft state as this possibly won't be needed Short-term as we opted to do this manually for now (see https://phabricator.wikimedia.org/T409428) Long-term the drop should probably happen either via Jobs/DeleteWikiDbJob.php or in a new dedicated, regularly scheduled pruning Job (see https://phabricator.wikimedia.org/T309588) |
Adds a
wbs-wiki:DropDatabaseCommandBug: T409936