Follow-up to #2568203: Remove migrate-db.sh in favor of core tools

The db-tools.sh import command just plain does not work to import a fixture into an external (specified on the command-line) database. It parses and adds the connection for the external database, but does not actually use it when importing. Simple fix!

Comments

mikeryan created an issue. See original summary.

phenaproxima’s picture

mikeryan’s picture

Issue tags: -blocker

It doesn't really block the other patch, it just means you can't use it to its fullest.

Status: Needs review » Needs work

The last submitted patch, fix-db-tools-import.patch, failed testing.

Status: Needs work » Needs review
phenaproxima’s picture

StatusFileSize
new1.76 KB
new2.35 KB

Have a test, yo. Let's get this in by RC.

The last submitted patch, 6: 2579399-6-FAIL.patch, failed testing.

The last submitted patch, 6: 2579399-6-FAIL.patch, failed testing.

Kazanir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

neclimdul’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/lib/Drupal/Core/Command/DbImportCommand.php
@@ -58,6 +58,8 @@ protected function execute(InputInterface $input, OutputInterface $output) {
+    Database::setActiveConnection($connection->getKey());

If you overwrite the global connection, we should reset it at the end. It should be as simple as something like this:

$old_key = Database::setActiveConnection($connection->getKey());
//code
Database::setActiveConnection($old_key);
phenaproxima’s picture

Status: Needs work » Needs review
StatusFileSize
new2.72 KB
new863 bytes

Done!

benjy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

neclimdul’s picture

Status: Reviewed & tested by the community » Needs review

Should have caught this the first review... sorry.

+++ b/core/modules/system/tests/src/Kernel/Scripts/DbImportCommandTest.php
@@ -56,18 +55,21 @@ class DbImportCommandTest extends KernelTestBase {
-    /** @var \Drupal\Core\Database\Connection $connection */
-    $connection = $this->container->get('database');
-    // Drop tables to avoid conflicts.
-    foreach ($this->tables as $table) {
-      $connection->schema()->dropTable($table);
-    }
+    $connection_info = array(
+      'driver' => 'sqlite',
+      'database' => ':memory:',
+    );
+    Database::addConnectionInfo($this->databasePrefix, 'default', $connection_info);

Should add @requires extension pdo_sqlite to this test method.

phenaproxima’s picture

StatusFileSize
new2.77 KB

Nice catch. All set!

neclimdul’s picture

Status: Needs review » Reviewed & tested by the community

Enabled and disabled pdo_sqlite locally and everything ran smoothly. Going to look into the previous failures but they're in "Drupal\toolbar\Tests\ToolbarAdminMenuTest" so its seems unrelated so RTBC'ing now.

neclimdul’s picture

I'm told their working on the toolbar hash failure over here. #2075889: Make Drupal handle incoming paths in a case-insensitive fashion for routing

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome-sauce!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 5132930 on 8.0.x
    Issue #2579399 by phenaproxima, mikeryan, neclimdul: db-tools.php import...

Status: Fixed » Needs work

The last submitted patch, 14: 2579399-14.patch, failed testing.

webchick’s picture

Status: Needs work » Fixed

PIFT = Pain In my Frigging Tush.

Status: Fixed » Closed (fixed)

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