Undefined array key "migration" in findMigrationDependencies()

WIP added to issue fork with extracted comment entity generator function. https://git.drupalcode.org/issue/wordpress_migrate-3390754 . then reroll after 📌 remove createPluginEntityFromPlugin in WordPressMigrationGenerator Needs review is committed.

That does not fix the problem but does isolate it within a much smaller function.

For anyone else debugging such stuff. with devel and symfony dumper turned on:

Adding the following to web/core/modules/migrate/src/Plugin/Migration.php circa line 745:

if (in_array($plugin_configuration['plugin'], ['migration', 'migration_lookup'], TRUE)) {
          if (!isset ($plugin_configuration['migration'])) {
            ddm('---- migration is not set on this plugin_configuration!');
            ddm($plugin_configuration, 'findmigrationdependencies plugin_configuration');
            ddm($process);
            ddm('--end---');
          } else {
            ddm('---- OK: migration is set on this plugin_configuration');
            ddm($plugin_configuration, 'findmigrationdependencies plugin_configuration');
            ddm($process);
            ddm('--OK end--- \n');
          }
          $return = array_merge($return, (array) $plugin_configuration['migration']);
        }

Getting closer to this!

Brought this to slack drupal #migrate and mikelutz gave useful info. i think this is the 'plugin derivatives' / plugin derivers' idea. Thus we should transition these migration config entities to 'derivers' when possible. (I think thats the same as derivatives.)

related examples: https://www.drupal.org/project/entity_timeline/issues/3281990 →
https://api.drupal.org/api/drupal/core!lib!Drupal!Component!Plugin!Deriv...
https://www.sitepoint.com/tutorial-on-using-drupal-8-plugin-derivatives-...
https://drupal.stackexchange.com/questions/293667/how-do-i-create-a-deri...
https://api.druphelp.com/api/drupal/core%21lib%21Drupal%21Component%21Pl...
https://www.drupal.org/docs/drupal-apis/plugin-api/plugin-derivatives →
https://manifesto.co.uk/drupal-plugin-api-examples-tutorial/
https://www.hashbangcode.com/article/drupal-9-creating-category-menu-usi...
https://www.adamfranco.com/2018/11/13/creating-a-drupal-plugin-system/

mikelutz: The quick and dirty fix here, without refactoring to derivatives is to put something in the template plugin for that key like migration: wordpress_comment so the plugin can load and pass it’s checks. It doesn’t quite matter much, it’s getting overwritten in the dynamically generated config entities anyway, but something needs to be there in the plugin template itself so it can be loaded and used to create the config entities.

The problem isn’t in the importer. You are generating valid config entities there. The module is trying to dynamically generate migrate_plus config entities using the migration plugins in the /migration folder as templates. It’s really not the right approach. If you want to derive multiple migrations from a template, you should use a plugin deriver similar to the way core derives node plugins by content type. The issue here is that your template migrations has https://git.drupalcode.org/project/wordpress_migrate/-/blob/8.x-3.x/migr...

  pid:
    plugin: migration_lookup
    source: comment_parent
    # migration ID generated dynamically

Despite the fact that this migration is only supposed to be a template, it’s a real valid plugin. And any version of createEntityFromPlugin creates an instance of this plugin to read the template from. Creating an instance of the plugin checks the dependencies. Checking the dependencies of the plugin scans it for migration lookup processes and adds the migration that you are looking up against to the dependencies. And it can’t because there is an undefined offset ‘migration’.

When using the migration_lookup process plugin, it is assumed that the migration configuration key is set. That defines the list of migrations used for the lookup operation. In migrations/wordpress_comment.yml there are currently two usages of migration_lookup that do not have the migration configuration defined (because they will be generated dynamically).

process:
  entity_id:
    plugin: migration_lookup
    source: post_id
    # migration generated dynamically
  pid:
    plugin: migration_lookup
    source: comment_parent
    # migration ID generated dynamically

In WordPressMigrationGenerator::createEntityFromPlugin there is a call to $plugin_manager->createInstance($plugin_id). In \Drupal\migrate\Plugin\MigrationPluginManager::createInstances there is a call to \Drupal\migrate\Plugin\Migration::getMigrationDependencies where the system tries to infer the required/optional dependencies for the migration plugin being created. Eventually \Drupal\migrate\Plugin\Migration::findMigrationDependencies is called when attempting to set a list of optional dependencies. In there, the each process plugins is analyzed. If usages of migration_lookup are found, the corresponding migration configuration key is added to the list of dependencies. When not set, the warning described in this issue is triggered.

  protected function findMigrationDependencies($process) {
	...
        if (in_array($plugin_configuration['plugin'], ['migration', 'migration_lookup'], TRUE)) {
          $return = array_merge($return, (array) $plugin_configuration['migration']);
        }
	...
  }

Back in \Drupal\wordpress_migrate\WordPressMigrationGenerator::createEntityFromPlugin there is also an explicit call to $migration_plugin->getMigrationDependencies(); which makes the warning to be triggered again.

Later on when processing the generated comment migration plugin, the missing migration keys are injected into the corresponding migration_lookup process definitions. Also, the migration dependencies are overridden.

        $process                                     = $migration->get('process');
        $process['entity_id'][0]['migration']        = $content_id;
        $process['comment_type'][0]['default_value'] = $storage->getSetting('comment_type');
        $process['pid'][0]['migration']              = $id;
        $process['field_name'][0]['default_value']   = $field_name;
        $migration->set('process', $process);
        $migration->set('migration_dependencies', ['required' => [$content_id]]);

At the end generating the configuration entities, the migration plugins have valid process sections. But at that point it is too late. The warnings have been triggered.

The current MR proposes to add the missing migration configuration in migrations/wordpress_comment.yml and set it to a _PLACEHOLDER_ which is later replaced by the proper value.

I tested the MR #23, and the migration warning is gone. But there is another warning still present.

Warning: Undefined array key "default_author" in Drupal\wordpress_migrate\WordPressMigrationGenerator->createMigrations() (line 112 of modules/contrib/wordpress_migrate/src/WordPressMigrationGenerator.php).

I will work on this.

An array key was being access before checking that it exists. I pushed a commit to fix this. It might be worth to create a follow up issue to make sure that all expected configurations exist or define a default value for them. Back to needs review.

thank you dinarcon and baltowen for unraveling that and catching the default_author problem. For similar catching problem entries, may want to see also #3123393: Users are duplicated → and 🐛 Better exception handling for if username does not exist Active 🐛 does not catch duplicate migration_group ID, missing file uri, or other duplicate migration plugins gracefully Needs work .

Really appreciate the help on both this and the new issue for adding config link.

i have also updated the "plan" today 🌱 Plan for wordpress_migrate 8.x-3.x beta release Active .

I had another look at the module to see how using derivatives would work. I think that should be addressed in a separate issue. For example, we could one "base migration" for all taxonomy migrations and that seems out of scope for this issue.

While investigating, I noticed the module is creating instances out of the "base migrations" shipped with the module. That seems unnecessary. Getting the plugin definition would suffice to accomplish what the \Drupal\wordpress_migrate\WordPressMigrationGenerator::createEntityFromPlugin method is expected to do. I guess this could be considered out of scope for this issue as well.

@hongpong what do you think about these remarks? And how would you like to proceed with this issue?

Took a step back. The source of the issue is having incomplete definitions for process plugins that leverage migration_lookup. An alternative solution to providing a placeholder for the missing migration configuration is to extract those process plugins out of the base migrations, as there are multiple examples already. MR 25 follows this approach. I also cherry-picked @baltowen's fix to default_author error.

Looking forward to feedback on both MRs. If we decide to go with #25, the topic of not creating base migration instances might be discussed in a follow up.

Increasing Priority, since it is placed under "Major issues" header in 🌱 Plan for wordpress_migrate 8.x-3.x beta release Active , so I am also adding that as parent issue.

Thank you dinarcon ressa baltowen. I would say we could open another issue about refactoring the migration entities - whether that is a deriver, placeholder values, base migrations or similar. However not sure how it should be titled.

I swear, I will get the merge request interface correctly for the commit messages someday (facepalm)!

I would note that I still get this issue, but I think it is not because of this issue: #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry → .

hongpong → closed merge request !23

It's so great to see all the activity and cooperation currently, and a steady move towards a 8.x-3.x beta release. So thank you all for that!

About crediting, don't give it another thought @hongpong. Like we agree, it was unclear ... I am not a coder, so what you are all doing, I could not, so please keep on doing that -- then I'll attempt to improve the documentation of WordPress Migrate -- and crediting :)

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