Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8476: Rename `phabricator_repository` database
Migrations applied cleanly, browsing around the Paste application seemed functional.
Diff Detail
- Repository
- rP Phabricator
- Branch
- master
- Lint
Lint Passed - Unit
Test Failures - Build Status
Buildable 6673 Build 6695: [Placeholder Plan] Wait for 30 Seconds
| Time | Test | |
|---|---|---|
| 0 ms | AphrontIsolatedDatabaseConnectionTestCase::testTransactionRollback | |
| 0 ms | AphrontMySQLDatabaseConnectionTestCase::testConnectionFailures | |
| 0 ms | AphrontIsolatedDatabaseConnectionTestCase::testDeletePermitted | |
| 0 ms | AphrontIsolatedDatabaseConnectionTestCase::testInsertGeneratesID | |
| 0 ms | AphrontIsolatedDatabaseConnectionTestCase::testIsolation | |
| View Full Test Results (2 Failed · 5 Passed) |
Event Timeline
This probably breaks these migrations:
20130801.pastexactions.php 20130805.pastemailkeypop.php
...which use PhabricatorPaste. If they run, the object will try to connect to _paste, but the data won't be there yet, since the migrations which later move it from _pastebin will not have run yet.
You can likely bin/storage upgrade --namespace test123 --no-quickstart to see them fail.
I think we have to deal with these on a case-by-case basis, which is a big part of why renaming phabricator_repository will be hard. Fortunately, these seem pretty easy:
- 20130801.pastexactions.php: I think we can just remove this. The data is not critical and the migration is almost 2 years old. Anyone who hasn't updated in the last 2 years can live without "x created this paste on such-and-such date." transactions.
- 20130805.pastemailkeypop.php: This is messy.
- We can remove it, then replace it with a similar new migration at HEAD. But this can only work because no intermediate migrations affect the object and the migration isn't particularly intrinsic to the object's state.
- We can update it in-place, but there's no way to connect to get a conn_w to the phabricator_pastebin database at HEAD. Particularly, when the migration executes, it will connect to phabricator_paste, which won't exist yet, then immediately fail. This seems complicated/hard in the general case, although maybe it's not that bad. We'd basically need a way to get a $conn to an arbitrary database by name.
| resources/sql/autopatches/20150609.pastebin.1.sql | ||
|---|---|---|
| 2 | Let Phabricator build this automatically by adding it to PhabricatorBuiltinPatchList instead. That will probably resolve the storage adjust issue. | |
I'm not convinced that it's a good idea, but in D13224 I toyed with the idea of allowing the "application name" to be override for this purpose.
Yeah, I saw D13224 after looking at this -- I think the general approach is probably fine, but I think we should expose some kind of explicit API to make it clear that some spooky black magic is occurring.
My ad-hoc DAO approach works with ./bin/storage upgrade --namespace test --no-quickstart, but currently fails on storage adjustment:
[2015-06-11 07:13:11] EXCEPTION: (PhutilInvalidStateException) Call setApplicationName() before calling getApplicationName()! at [<phabricator>/src/infrastructure/storage/lisk/PhabricatorAdHocDAO.php:9] arcanist(head=master, ref.master=b70c7d7fc59e), phabricator(head=master, ref.master=bd2834fa1912), phutil(head=master, ref.master=92882eb9404d) #0 PhabricatorAdHocDAO::getApplicationName() called at [<phabricator>/src/applications/config/schema/PhabricatorConfigSchemaSpec.php:52] #1 PhabricatorConfigSchemaSpec::buildLiskObjectSchema(PhabricatorAdHocDAO) called at [<phabricator>/src/applications/config/schema/PhabricatorConfigCoreSchemaSpec.php:21] #2 PhabricatorConfigCoreSchemaSpec::buildSchemata(PhabricatorConfigServerSchema) called at [<phabricator>/src/applications/config/schema/PhabricatorConfigSchemaQuery.php:172] #3 PhabricatorConfigSchemaQuery::loadExpectedSchema() called at [<phabricator>/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php:33] #4 PhabricatorStorageManagementWorkflow::loadSchemata() called at [<phabricator>/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php:352] #5 PhabricatorStorageManagementWorkflow::findAdjustments() called at [<phabricator>/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php:46] #6 PhabricatorStorageManagementWorkflow::adjustSchemata(boolean, boolean, boolean) called at [<phabricator>/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php:222] #7 PhabricatorStorageManagementUpgradeWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:406] #8 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:301] #9 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/sql/manage_storage.php:176]
Fix storage adjustment
| src/infrastructure/storage/lisk/PhabricatorAdHocDAO.php | ||
|---|---|---|
| 20 ↗ | (On Diff #32002) | I'll tidy this up and document things a bit better if we decide on this approach. |
Although it mostly works, I am abandoning this diff because keeping it up-to-date seems like more effort than I am willing to give. It's not clear to me at this stage when (if ever) T8476 is likely to be resolved.