Problem/Motivation

To generate database fixtures for testing, Migrate currently relies on core/scripts/migrate-db.sh. This script was created before Symfony's Console component was put into core, and DumpDbApplication was written -- for the same purpose. There should not be two ways to dump a database in Drupal core.

Proposed Resolution

Migrate should use the DumpDbAppliction to generate its database fixtures, and migrate-db.sh should be removed. @neclimdul already started down this path in #2550291: Improve and generalize database dump tools.

Remaining Tasks

  1. DumpDbApplication must be extended so that it can dump any database defined in settings.php.
  2. Migrate Drupal's D6 and D7 database fixtures need to be re-generated using the extended DumpDbAppliation.
  3. Migrate Drupal's base test classes may need to be changed so that their loadDumps() methods create the test database from the generated fixtures.
  4. migrate-db.sh must be removed.
  5. This might sound like a lot, but it's really just one patch to review and commit.

API/UI Changes

No API changes or UI changes, but there is a DX change, which is that changes to the D6 and D7 database fixtures must be exported using DumpDbApplication.

CommentFileSizeAuthor
#7 d6_dump_updates.txt68.45 KBmikeryan
#4 2568203-4.patch4.17 MBphenaproxima
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Priority: Normal » Major
Status: Active » Postponed

This is postponed on #2550291: Improve and generalize database dump tools and major for the same reason.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Postponed » Active
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Active » Needs review
FileSize
2568203-4.patch4.17 MB

This patch:

  • Is over 4 MB. I'm so sorry!
  • Removes migrate-db.sh, in favor of core/scripts/db-tools.php.
  • Removes the D6 and D7 dump files generated by migrate-db.sh, replacing them with the files in core/modules/migrate_drupal/tests/fixtures. The fixtures are uncompressed because, if they were compressed, it would be excruciating to reroll any patch that needed to change the data in either fixture, and git merges would be impossible. I think we should leave the fixtures uncompressed until Migrate and Migrate Drupal are stripped of their "experimental" status (i.e., Drupal 8.1), at which point we should seal the dumps by gzipping them. The tests already support compression, so that patch will take about 2 minutes to roll.
  • Removes the base class used by the migrate-db.sh dump files.
  • Removes the test which verified the hashes of the migrate-db.sh dump files.
  • Alters the tests' base classes to load the new fixtures. MigrateDrupalTestBase::loadDumps() was no longer needed, so I iced it.

All Drupal 6 tests passed on my localhost before uploading this, so I'm crossing my fingers. I really do not want to re-roll this.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented

The code looks good to me - I'd like to try actually using it to extract the dump, modify it, and redump though. Not sure if I'll have time today.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Postponed

Blocked by #2579399: db-tools.php import does not work.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Postponed » Reviewed & tested by the community
FileSize
d6_dump_updates.txt68.45 KB

That issue doesn't really block this one - this patch does exactly what it should do, given a D6 db out there. To manually test it I did (with the import fix in place):

  1. Emptied my d6_dump database.
  2. php core/scripts/db-tools.php import --database-url mysql://root@127.0.0.1:33067/d6_dump core/modules/migrate_drupal/tests/fixtures/drupal6.php
  3. Manually added a uid 0 row to the users table (would be nice if we could hack that in there automatically)
  4. Logged in to a D6 site configured to point at d6_dump as root/root.
  5. Ran update.php, because our dump data is out-of-date.
  6. php core/scripts/db-tools.php dump --database-url mysql://root@127.0.0.1:33067/d6_dump >core/modules/migrate_drupal/tests/fixtures/drupal6.php
  7. git diff >~/tmp/d6_dump_updates.txt

Attached is a patch file to update our D6 fixture to reflect current releases of D6 core and CCK. The process works as intended (and better than the old process - for one thing, now the cache tables have proper longblobs).

It'll be good to follow this up with #2578483: [meta] Keep dumps up-to-date and improve DX...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Thanks a lot for the extensive manual testing, Mike! I think this step makes sense to commit on its own, esp. given the pain in the ass factor to re-roll it.

However, we might need to roll back this whole DB Tools circus if we end up in danger of regressing from beta16 in rc1. Fortunately, we still have a little bit of time to clean this up before then.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 00d4deb on 8.0.x 
    Issue #2568203 by phenaproxima, mikeryan: Remove migrate-db.sh in favor...

Status: Fixed » Needs work

The last submitted patch, 4: 2568203-4.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Fixed

PIFT you're bringing me down.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.