Page MenuHomePhabricator

Rename Paste database
AbandonedPublic

Authored by joshuaspence on Jun 9 2015, 10:03 AM.
Tags
None
Referenced Files
F14474887: D13223.id.diff
Fri, Dec 27, 4:52 PM
Unknown Object (File)
Tue, Dec 24, 8:34 PM
Unknown Object (File)
Sun, Dec 22, 1:58 PM
Unknown Object (File)
Sun, Dec 22, 12:45 AM
Unknown Object (File)
Tue, Dec 3, 2:57 PM
Unknown Object (File)
Sun, Dec 1, 5:10 AM
Unknown Object (File)
Sat, Nov 30, 10:32 PM
Unknown Object (File)
Thu, Nov 28, 1:21 AM

Details

Summary

Ref T8476. Rename the Paste database from phabricator_pastebin to phabricator_paste, for consistency with other applications.

Test Plan

Migrations applied cleanly, browsing around the Paste application seemed functional.

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6642
Build 6664: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Rename Paste database.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

I'm not sure why, but ./bin/storage adjust complains about something:

paste.png (744×1 px, 102 KB)

epriestley edited edge metadata.

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
1

Let Phabricator build this automatically by adding it to PhabricatorBuiltinPatchList instead. That will probably resolve the storage adjust issue.

This revision now requires changes to proceed.Jun 10 2015, 2:15 PM

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.

joshuaspence edited edge metadata.
joshuaspence marked an inline comment as done.

Fix ./bin/storage adjust issues

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.

joshuaspence edited edge metadata.

Here's another approach... exposing an "adhoc" DAO for this purpose.

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.