$ drush updb
The following updates are pending:

 migrate module                                                                 
 6021 - Update map tables to reflect change of needs_update to a status column. 

Do you wish to run all pending updates? (y/n): y
Executing migrate_update_6021                                          
Invalid argument supplied for foreach() database.inc:1452           

After this, I get a crash dump with the following message:
Exception: DB does not exist in Database::openConnection() (line 1541 of docroot/sites/all/modules/contrib/dbtng/database/database.inc

I updated to latest dev of dbtng, and this now works, but then I get another error.

drush updb -y 
The following updates are pending:

 migrate module                                                                 
 6021 - Update map tables to reflect change of needs_update to a status column. 

Do you wish to run all pending updates? (y/n): y
Executing migrate_update_6021                                          

Class MyMigration no longer exists                                   [error]
WD php: MigrateException: Failure to sort migration list - most likely [error]
due to circular dependencies involving
[comma separated list of my other migrations]
in migrate_migrations() (line 74 of docroot/sites/all/modules/contrib/migrate/migrate.module

So clearly, there must be a reference to my "MyMigration" class somewhere in my code, but after running grep -Rn I was unable to track down any reference to it. This was an old migration that I have disabled, and all dependency references to it have been disabled.

I could execute migrate_migrations() before updating to migrate 2.3 dev, so there appears to be another methodology in play for assembling and sorting the list.

Looking at migrate_migrations(), and comparing to my old version, I see this:

 * Retrieve a list of all active migrations, ordered by dependencies. To be
 * recognized, a class must be non-abstract, and derived from MigrationBase.

I also note that now the list is built from a list of migrations in the migrate_status table rather than from the autoload table.

This means out of date, inactive classes will appear in the list (since out of date and disabled migrations will still have a record in the migrate_status table.

The list should really be derived from the autoload table, since otherwise migrate_migrations() will continue to look for classes due simply to their appearance as a row in the migrate_status table rather than their appearance in the dynamic (cached) rows of the autoload_* tables.

Comments

apotek’s picture

I removed the out of date MyMigration class from the migrate_status table and was able to get much farther along in my updb command, but the basic failure still applies. migrate_migrations() is dying.

If I revert to the previous version of migrate, I can run drush php-eval '$ms = migrate_migrations();' which no issues whatsoever. What has changed in terms of how the dependencies are checked?

$ drush updb -y
The following updates are pending:

 dbtng module                              
 6001 - Add the new dbtng_sequences table. 


 migrate module                                                                   
 6002 - Add support for history logging                                           
 6003 - Add and populate class_name field, and add arguments field. Any existing  
 migration code using dependencies or sourceMigration() must be changed!  See     
 CHANGELOG.txt.                                                                   
 6020 - We messed up, by starting Migrate V2 update functions at 6001 - Migrate   
 V1 on D6  was up to 6016. We restart at 6020 making sure the V2 tables get       
 created and  giving some advice on upgrading.                                    
 6021 - Update map tables to reflect change of needs_update to a status column.   


Do you wish to run all pending updates? (y/n): y
Executing dbtng_update_6001                                            [success]
Executing migrate_update_6002                                          [success]
Table 'migrate_log' already exists                           [warning]
query: CREATE TABLE migrate_log (
`mlid` INT unsigned NOT NULL auto_increment, 
`machine_name` VARCHAR(255) NOT NULL, 
`process_type` TINYINT unsigned NOT NULL, 
`starttime` BIGINT unsigned NOT NULL, 
`endtime` BIGINT unsigned DEFAULT NULL, 
`initialhighwater` VARCHAR(255) NOT NULL, 
`finalhighwater` VARCHAR(255) DEFAULT NULL, 
`numprocessed` INT unsigned DEFAULT NULL, 
PRIMARY KEY (mlid)
) /*!40100 DEFAULT CHARACTER SET utf8 */ database.mysqli.inc:147
CREATE TABLE {migrate_log} (                                           [error]
`mlid` INT unsigned NOT NULL auto_increment, 
`machine_name` VARCHAR(255) NOT NULL, 
`process_type` TINYINT unsigned NOT NULL, 
`starttime` BIGINT unsigned NOT NULL, 
`endtime` BIGINT unsigned DEFAULT NULL, 
`initialhighwater` VARCHAR(255) NOT NULL, 
`finalhighwater` VARCHAR(255) DEFAULT NULL, 
`numprocessed` INT unsigned DEFAULT NULL, 
PRIMARY KEY (mlid)
) /*!40100 DEFAULT CHARACTER SET utf8 */
ALTER TABLE {migrate_status} DROP lastthroughput                       [success]
ALTER TABLE {migrate_status} DROP lastimportedtime                     [success]
Executing migrate_update_6003                                          [success]
ALTER TABLE {migrate_status} ADD `class_name` VARCHAR(255) NOT NULL    [success]
DEFAULT ''
ALTER TABLE {migrate_status} ADD `arguments` LONGBLOB DEFAULT NULL     [success]
Executing migrate_update_6020                                          [success]
Executing migrate_update_6021                                          [success]
Origin addFieldMapping: field_vanity_url was previously mapped,        [warning]
overridden
WD php: MigrateException: Failure to sort migration list - most likely [error]
due to circular dependencies involving
FooMigration,BarMigration,ZooMigration
in migrate_migrations() (line 74 of
/home/kwidholm/platforms/mags/docroot/sites/all/modules/contrib/migrate/migrate.module).
Drush command terminated abnormally due to an unrecoverable error.     [error]

apotek’s picture

I believe this issue is going to be a blocker for a fair amount of people who are updating from previous migrate module versions.

mikeryan’s picture

Status: Active » Postponed (maintainer needs more info)

migrate_migrations() does

    if (class_exists($row->class_name)) {

for each migration retrieved from the migrate_status table - this is why it reports to you that the MyMigration class does not exist. This much is working as it should, most likely MyMigration has nothing to do with the apparent circular dependency it's detecting among the other migrations.

Are you absolutely sure that the migrations it's reporting on are both in the migrate_status table and the autoload_registry table? I'm wondering if it's an autoload issue - does adding

  // Make sure autoload registers our classes
  drupal_flush_all_caches();

to the beginning of migrate_update_6021() help?

Thanks.

apotek’s picture

Mike, I think the issue is that the new way of discovering migrations relies on which migrations are *present* in the migrate_status table, regardless of whether they are still active. In my migrate_status table, I have a migration 'foo' that used to be active, but which has been deactivated by being removed from the files[] array in the my_migration.info file. I have also done a `grep -Rn 'FooMigration' [my migration module]` and made sure I am making no more references to the FooMigration module in any active migrations.

To answer your questions

$ drush php-eval '
drupal_flush_all_caches();
$result = db_query("SELECT class_name from migrate_status"); while ($row = db_fetch_object($result)) {
> if (class_exists($row->class_name)) {
> echo "Class " . $row->class_name . " exists\n";
> } else {
> echo "Class ". $row->class_name . " does not exist.\n";
> }
> }
> '
Class FooMigration exists
Class BarMigration exists
Class YahooMigration exists
Class DeactivatedMigration does not exist
...

All active migrations are reported as existing. The migration I commented out of the files[] array in my_migrate_module.info is correctly listed as not existing any more. But of course, the migration still exists in the migrate_status table.

If I leave the DeactivatedMigration migration in the migrate_status table, these are my results.

$ drush updbThe following updates are pending:

 migrate module                                                                 
 6021 - Update map tables to reflect change of needs_update to a status column. 

Error
MigrateException: Failure to sort migration list - most likely due to circular dependencies involving FooMigration, BarMigration, YahooMigration in <em>migrate_migrations()</em> (line <em>74</em> of sites/all/modules/contrib/migrate/migrate.module)

If I remove the DeactivatedMigration migration from the migrate_status table, these are my results

$ drush updbThe following updates are pending:

 migrate module                                                                 
 6021 - Update map tables to reflect change of needs_update to a status column. 

Error
MigrateException: Failure to sort migration list - most likely due to circular dependencies involving FooMigration, BarMigration, YahooMigration in <em>migrate_migrations()</em> (line <em>74</em> of sites/all/modules/contrib/migrate/migrate.module)

Same result in either case.

There are no circular dependencies. I run my migrations every day. I can run migrate-status with no problem. As noted above, there are no references in any active migrations any longer to the DeactivatedMigration.

The DeactivatedMigration, because it was removed from files[] does not show up in the Autoload tables. This would be expected behavior.

We are trying to update from 6.x-2.x-dev, 2010-11-11.

So there are two factors here that I think might be of interest.

1. That a "circular dependency" error is being reported, when in fact all my migrations can run normally in day to day use.

2. Perhaps the deactivated module is a red herring at this point. It seems like whether it exists in the migate_status table or not, the error I get is the same.

3. Another interesting tidbit. There are two levels of abstraction for my migrations. In other words, my_migration_module defines its classes like this:

abstract class MyAbstractBaseMigration extends Migration {
// general stuff shared by all site implementations and all migrations.
}

And then I create abstract migrations for all migrations shared by all sites:

abstract class FooAbstractMigration  extends MyAbstractBaseMigration {
// general stuff shared by all migrations for site N
}

And then for each site, we finally implement the real class:

class FooMigration extends FooAbstractMigration {
// usually next to nothing in here
}

This hierarchy is working perfectly for actually moving data around and letting me make small adjustments to each migration per site, while keeping 99% of the code in the abstract classes (in one place).

But perhaps there are assumptions in the migrate_migrations() code now, that would break this kind of multi-level hierarchy?

apotek’s picture

I decided to simplify things.

Here's some oddness to chew on.

 $ drush php-eval '
$ms = migrate_migrations();
var_dump($ms);
echo "\n";'
Class DeactivatedMigration no longer exists                                   [error]
WD php: MigrateException: Failure to sort migration list - most likely [error]
due to circular dependencies involving
Foo,Bar,Yahoo in migrate_migrations() (line 74 of
/home/kwidholm/platforms/mags/docroot/sites/all/modules/contrib/migrate/migrate.module).

A couple things I note: 1. That DeactivatedMigration is referred to by its real name with Migration attached to the end. 2. The DeactivatedMigration does not appear in the list of all my active migrations on the following line "dependencies involving ......". 3. The list of migrations in the 'dependencies involving.... ' output are not called FooMigration, BarMigration etc etc, but simply Foo, Bar.

It's as if the class names have had the 'Migration' part chopped off.

apotek’s picture

Okay, I think the DeactivatedMigration is a red herring.

I did some crude analysis using echo :-)

(lines 82:86)...

    foreach ($dependent_migrations as $name => $migration) {
echo "checking dependencies for migration $name\n";
      $ready = TRUE;
      // Scan all the dependencies for this class and make sure they're all
      // in the final list
      foreach ($migration->getDependencies() as $dependency) {
echo "\tdependency: $dependency";
        if (!isset($migrations[$dependency])) {
echo "\t NOT READY\n";
          $ready = FALSE;
          break;
        }
      }
      if ($ready) {
echo "\t READY\n";

In the output, NONE of my dependencies were ever marked as READY.

In other words, the migrate_migrations() is failing not because of any specific migration dependency, but because the dependency checking is broken in some way, and it keeps iterating up until the artificial limit of 20, at which point it fires off the error.

I should also say that the contents of the $migrations variable prior to the while loop in line 75 is populated with 4 migrations, none of which declare any hard dependencies in their protected Dependencies variable.

I have 12 dependent migrations.

I upped the iterations limit to 100, and I still never got a single message where a dependency was marked as ready.

apotek’s picture

Ok, Here's the problem.

The $migrations array keys are class names WITHOUT the Migration appended, ie Foo, Bar, etc, NOT FooMigration, BarMigration.

This is also the case with $dependent_migrations.

However, the result of $migration->getDependencies() returns FooMigration, BarMigration, YooMigration. In other words, we are checking for keys that don't ever ever exist, because the migrations in the Migration::Dependencies() variables always incluce 'Migration' in their names, whereas the migrations in $migrations and $dependent_migrations don't.

At some point, what this function *think* is the class_name (FooMigration) becomes actually the machine name of the migration (Foo). but since dependencies are declared with Class Name, the dependencies can never be resolved.

Example, line 48 looks like so:

          $dependent_migrations[$row->machine_name] = $migration;

I would think, since it's going to be compared later, to class names, not machine names, that it should look like this:

          $dependent_migrations[$row->class_name] = $migration;

Or has the API changed such that we are now supposed to be declaring MyMigration::Dependencies() using machine names rather than the class names? ie:

self::Dependencies = array('MyMachineName', 'MyMachineName2')

instead of

self::Dependencies = array('MyClassName', 'MyClassName2')
apotek’s picture

I changed lines 46:55 from

        if (count($dependencies) > 0) {
          // Set classes with dependencies aside for reordering
          $dependent_migrations[$row->machine_name] = $migration;
          $required_migrations += $dependencies;
        }
        else {
          // No dependencies, just add
          $migrations[$row->machine_name] = $migration;
        }
      }

to:


        if (count($dependencies) > 0) {
          // Set classes with dependencies aside for reordering
          $dependent_migrations[$row->class_name] = $migration;
          $required_migrations += $dependencies;
        }
        else {
          // No dependencies, just add
          $migrations[$row->class_name] = $migration;
        }
      }

And migrate_migrations() seems to work now. If I'm right about making the class names the keys, I'll create a patch. But if you have decided that dependencies should be specified by machine name, I'll have to change my code and hope for the best elsewhere ( :-! )

mikeryan’s picture

Dependencies have been based on machine name rather than class name for over a year now: #989200: Dynamic migrations - looking at CHANGELOG.txt, this has been in there since Migrate 2.0 Beta 3. But, the question is, how can it break here if calls to migrate_migrations() work for you elsewhere - e.g., drush ms, drush mi --all, etc...?

apotek’s picture

Ok. I'm going to go through and update my migrate implementations to use the bare migration names without the Migration part. Thanks.

mikeryan’s picture

Status: Postponed (maintainer needs more info) » Closed (works as designed)