Problem/Motivation

Trying to run any migration operation with the Drush integration of Migrate Tools causes a fatal error. It appears that it might be as simple as an incorrect use statement. While I found this via drush, I expect this probably would cause a fatal error for any migration at all, as soon as the source plugin manager attempts to instantiate the Disqus source plugin, it will fail.

TypeError: Argument 5 passed to Drupal\disqus\Plugin\migrate\source\DisqusComment::create() must be an instance of                     [error]
Drupal\migrate\Entity\MigrationInterface, instance of Drupal\migrate\Plugin\Migration given, called in
/var/www/build/html/core/modules/migrate/src/Plugin/MigratePluginManager.php on line 57 in
Drupal\disqus\Plugin\migrate\source\DisqusComment::create() (line 79 of
/var/www/build/html/modules/contrib/disqus/src/Plugin/migrate/source/DisqusComment.php) #0
/var/www/build/html/core/modules/migrate/src/Plugin/MigratePluginManager.php(57):
Drupal\disqus\Plugin\migrate\source\DisqusComment::create(Object(Drupal\Core\DependencyInjection\Container), Array, 'disqus_source',
Array, Object(Drupal\migrate\Plugin\Migration))
#1 /var/www/build/html/core/modules/migrate/src/Plugin/Migration.php(335):
Drupal\migrate\Plugin\MigratePluginManager->createInstance('disqus_source', Array, Object(Drupal\migrate\Plugin\Migration))
#2 /var/www/build/html/modules/contrib/migrate_tools/migrate_tools.drush.inc(471): Drupal\migrate\Plugin\Migration->getSourcePlugin()
#3 /var/www/build/html/modules/contrib/migrate_tools/migrate_tools.drush.inc(147): drush_migrate_tools_migration_list('')
#4 /var/www/vendor/drush/drush/includes/command.inc(422): drush_migrate_tools_migrate_status()
#5 /var/www/vendor/drush/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#6 /var/www/vendor/drush/drush/includes/command.inc(199): drush_command()
#7 /var/www/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#8 /var/www/vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#9 /var/www/vendor/drush/drush/drush.php(12): drush_main()
#10 {main}.

Proposed resolution

Change the use statement, see if that helps it.

Remaining tasks

All the things

User interface changes

None expected.

API changes

None expected.

Data model changes

None expected.

CommentFileSizeAuthor
#25 2879592-25.patch14.89 KBnuuou
#24 interdiff.txt17.31 KBgaurav.kapoor
#24 2879592-24.patch13.95 KBgaurav.kapoor
#18 remove-migration-scripts-2879592-18.patch26.25 KBjason.blanda
#14 interdiff.txt1.14 KBmarvil07
#14 disqus-migrate-support-2879592-14.patch8.57 KBmarvil07
#11 disqus-migrate-support-2879592-11.patch7.66 KBsweetchuck
#9 interdiff.txt1.85 KBdylan donkersgoed
#9 disqus-migrate-support-2879592-9.patch6.63 KBdylan donkersgoed
#8 interdiff.txt4.07 KBdylan donkersgoed
#8 disqus-migrate-support-2879592-7.patch6.21 KBdylan donkersgoed
#6 disqus-migrate-support-2879592-6.patch2.54 KBdylan donkersgoed
#4 suppress_disqus_migrate_support-2879592-3-do-not-test.patch20.06 KBGrayside
#2 suppress_disqus_migrate_support-2879592-2-do-not-test.patch20.06 KBGrayside

Comments

Grayside created an issue. See original summary.

Grayside’s picture

Grayside commented
StatusFileSize
new suppress_disqus_migrate_support-2879592-2-do-not-test.patch20.06 KB

Correcting the use statement to use Drupal\migrate\Plugin\MigrationInterface; from use <code>Drupal\migrate\Entity\MigrationInterface; surfaced another error:

TypeError: Argument 2 passed to t() must be of the type array, string given, called in                                                 [error]
/var/www/build/html/modules/contrib/disqus/src/Plugin/migrate/source/DisqusComment.php on line 149 in t() (line 304 of
/var/www/build/html/core/includes/bootstrap.inc) #0
/var/www/build/html/modules/contrib/disqus/src/Plugin/migrate/source/DisqusComment.php(149): t('There was an er...', 'error')
#1 /var/www/build/html/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php(244):
Drupal\disqus\Plugin\migrate\source\DisqusComment->initializeIterator()
#2 /var/www/build/html/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php(454):
Drupal\migrate\Plugin\migrate\source\SourcePluginBase->getIterator()
#3 /var/www/build/html/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php(414):
Drupal\migrate\Plugin\migrate\source\SourcePluginBase->doCount()
#4 /var/www/build/html/modules/contrib/migrate_tools/migrate_tools.drush.inc(185):
Drupal\migrate\Plugin\migrate\source\SourcePluginBase->count()
#5 /var/www/vendor/drush/drush/includes/command.inc(422): drush_migrate_tools_migrate_status()
#6 /var/www/vendor/drush/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#7 /var/www/vendor/drush/drush/includes/command.inc(199): drush_command()
#8 /var/www/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#9 /var/www/vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#10 /var/www/vendor/drush/drush/drush.php(12): drush_main()
#11 {main}.

Since I am not using this migration plugin, I have attached a horrible patch that removes all the migrate plugins so I can move forward without this. Sorry not to be a better collaborator in this instance.

tobby’s picture

tobby commented

FWIW this is probably related to a change in Migrate Plus, where they changed how migrate plugins are discovered and to use Core's Migrate plugin manager. See https://www.drupal.org/node/2752335 for the relevant change. This Disqus migrate plugin doesn't throw an automatic fatal error when using older versions of Migrate Plus and Migrate Tools (~8.x-2.x, which is intended for Drupal 8.1). This particular issue is going to pop up for later versions, like Migrate Plus and Migrate Tools 8.x-4.x, which is intended for Drupal 8.3+.

Grayside’s picture

Grayside commented
StatusFileSize
new suppress_disqus_migrate_support-2879592-3-do-not-test.patch20.06 KB

That one had my use statement correction breaking the patch.

Anonymous’s picture

Anonymous (not verified) commented

For anyone facing this issue, the fix is to remove migrations from Disqus. You will need to delete the following directory in the disqus module : src/Plugin/migrate and of course, run drush cr.

dylan donkersgoed’s picture

dylan donkersgoed commented
StatusFileSize
new disqus-migrate-support-2879592-6.patch2.54 KB

I encountered a similar issue when trying to run a migration of D6 comments to Disqus comments for a D8 site. The destination plugin is actually already pretty close to working and I'm attaching a patch to get it running. It:

- adds a logger channel service
- updates the use statement etc. for the logger accordingly
- implements ContainerFactoryPluginInterface so dependencies can be injected

Seems to work pretty well so far.

It's my understanding that the last couple patches are removing the migrations as a temporary fix so I haven't included them in this patch.

Grayside’s picture

Grayside commented

Dylan, that is correct. I wasn't trying to perform a Disqus migration, just have the module integrated into a site in which a migration needed to be run. As such, I posted a patch to suppress the migration and left the real work to someone who needed it. Thank you for contributing a more helpful step to a solution.

dylan donkersgoed’s picture

dylan donkersgoed commented
StatusFileSize
new disqus-migrate-support-2879592-7.patch6.21 KB
new interdiff.txt4.07 KB

I've expanded my patch from above to include mapping of Disqus IDs in the destination plugin and rollback functionality. It should now be possible to rollback disqus migrations and map parent comment ids from migrations.

dylan donkersgoed’s picture

dylan donkersgoed commented
StatusFileSize
new disqus-migrate-support-2879592-9.patch6.63 KB
new interdiff.txt1.85 KB

New patch, added support for migrating comments to Disqus guest comments. They need to be approved afterwards unfortunately - seems like you should be able to approve them via the API afterwards but it doesn't seem to work with a quick test. Also made errors a bit clearer. Unfortunately date and IP can't be migrated this way.

sittard’s picture

sittard commented

Please could we have this committed and fixed. Thanks.

sweetchuck’s picture

sweetchuck
Primary language Hungarian
Location Budapest
commented
Status: Active » Needs review
StatusFileSize
new disqus-migrate-support-2879592-11.patch7.66 KB
dylan donkersgoed’s picture

dylan donkersgoed commented

Before this gets committed I want to note here that I ended up giving up on performing a migration via the module altogether due to some limitations in Disqus's API. Notably I never could get the date and IP to migrate properly (likely wasn't actually possible via the API) and got constant lockouts due to the large number of queries. Instead I wrote a one-off drush script to generate a few XML files which I imported via Disqus's migration tool. Unfortunately that won't work with Drupal's migration framework.

So before you use my patch remember that even I didn't in the end, though it should work in some circumstances.

sittard’s picture

sittard commented

Yeah I'm in a agreement.

At the moment Disqus RC3 is blocking our site migration (we don't even need to migrate Disqus comments).

As this patch is partly untested would it be possible to have a working Disqus RC4 released with the migration elements removed. Please can migration and this issue be moved to the development branch until it's fully tested and working.

marvil07’s picture

marvil07 commented
StatusFileSize
new disqus-migrate-support-2879592-14.patch8.57 KB
new interdiff.txt1.14 KB

I am having problems with the code on the current release when I try to execute drush migrate-status.
I see that some of the problems are already fixed on the latest patch here, so I went ahead and updated the latest patch to fix the problem I see, but I have not really reviewed the whole patch.

frodeste’s picture

frodeste commented

Can someone commit the patch?

abu-zakham’s picture

abu-zakham commented
Status: Needs review » Reviewed & tested by the community
ludo.r’s picture

ludo.r
Location Brussels
commented

I applied the patch #14 and it solved the intial error when doing a drush migrate-status. I'm using Disqus 8.x-1.0-rc3

jason.blanda’s picture

jason.blanda commented
StatusFileSize
new remove-migration-scripts-2879592-18.patch26.25 KB

If you are not in need of the Disqus migration files but do need to run other migrations here is a patch to remove Disqus migration files and templates.

robloach’s picture

robloach
he/him
commented

Thanks! Perhaps we should consider moving the Migration scripts to a separate module if it's causing issue. disqus_migrate or similar?

In the mean time, it sounds like https://www.drupal.org/files/issues/disqus-migrate-support-2879592-14.patch is good to go....

gaurav.kapoor’s picture

gaurav.kapoor commented
Assigned: Unassigned » gaurav.kapoor
Webbeh’s picture

Webbeh
he/him/his
Location Georgia, USA
commented
Assigned: gaurav.kapoor » Unassigned

Looks like this is RTBC from feedback, un-assigning #20.

gaurav.kapoor’s picture

gaurav.kapoor commented
Status: Reviewed & tested by the community » Needs work

The patch provided in #14 would require a reroll and changes related to D9 compatibility included.

gaurav.kapoor’s picture

gaurav.kapoor commented
Issue tags: +Needs reroll
gaurav.kapoor’s picture

gaurav.kapoor commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 2879592-24.patch13.95 KB
new interdiff.txt17.31 KB

Rerolled the patch as per latest release as well as resolved all the errors which were shown when trying to run drush ms. Fixed CS issues in destination and source plugin.

nuuou’s picture

nuuou commented
StatusFileSize
new 2879592-25.patch14.89 KB

Modified the above patch to also include some changes from a related issue, as they conflicted:
https://www.drupal.org/project/disqus/issues/3089234

Without both of these, I was unable to run "drush ms" or perform any migration-related tasks.

  • Nuuou authored 0eea81b on 8.x-1.x 
    Issue #2879592 by Dylan Donkersgoed, Grayside, gaurav.kapoor, marvil07,...
gaurav.kapoor’s picture

gaurav.kapoor commented

After testing, 4-5 different scenarios I believe we should commit patch in #25 and unblock things for now and in long run we can work on moving migration related logic to a different module. There is a still a warning message shown when API keys aren't configured but that is still better creating a migration blocker in all the projects which use this module. Feel free to open this issue again if someone has different thoughts.

gaurav.kapoor’s picture

gaurav.kapoor commented
Status: Needs review » Fixed

Thanks everyone for working on this.

Status: Fixed » Closed (fixed)

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