There's all sorts of issues with FeedsMissingPlugin and things like #2379223: Disable gave white screen! which could be avoided if we made the modules that provide plugins required if their plugins are in use.

CommentFileSizeAuthor
#1 feeds-module-required-2379407-1.patch1.93 KBtwistor
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

twistor’s picture

Status: Active » Needs review
FileSize
1.93 KB
MegaChriz’s picture

Great addition!

I tested with a few plugins and it seemed to work very well: I was no longer able to disable a module that provided a Feeds plugin if that plugin was used in a importer (regardless of the importer in question was enabled or disabled and if the importer was in the db or in code). I also checked if the module couldn't be disabled through Drush.

Use static cache?

The function feeds_system_info_alter() will be called for every module on the system whenever the module admin page is visited, so it will be called a lot. I wonder if we should use static cache for the loaded Feeds importers? I did notice there is some static cache further in the called functions (ctools_get_plugins(), ctools_export_load_object(), FeedsConfigurable::instance()), so maybe there is only little gain.

Other causes of FeedsMissingPlugin

Whenever I run into FeedsMissingPlugin issues most times is not by accidentally disable a module that provided a plugin that was in use, but when I enable a module created with the Features module that contains a Feeds importer. In such case I forgot to list all the dependencies that were needed for the Feeds importer when creating the feature, as the dependencies are not automatically detected when putting a Feeds importer in a feature. This is a probably food for an other issue, though.

  • twistor committed b860e72 on 7.x-2.x
    Issue #2379407 by twistor, MegaChriz: Make module required if plugins...
twistor’s picture

Assigned: twistor » Unassigned
Status: Needs review » Fixed
Issue tags: -Needs tests

Nothing is loaded unless the module implements hook_feeds_plugins(), which should be a few number of modules.

All of those objects are statically cached someplace else anyway.

Feel like creating that Features issue :)

MegaChriz’s picture

Ah, I overlooked that first line. Then there is not much point introducing extra static cache indeed.

I put creating the Features issue on my to do list (I should search for existing issues first). Will report here as soon the issue exists.

MegaChriz’s picture

I created the follow-up issue for the Features issue I mentioned in #2: #2387419: Auto detect dependencies when putting Feeds importer in a feature.

Status: Fixed » Closed (fixed)

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