Hm hopefully this isn't a red herring. This site has a bit of a sketchy history. Anyway, trying to install Feeds 7.x-2.0-beta1. Previous version was 7.x-2.0-alpha7+18-dev with two patches (one for http://drupal.org/node/661606#comment-6857198 and one for http://drupal.org/files/feeds-nocreate-1286298-7.patch).
Upon trying to run the associated database updates, fails on
PHP Fatal error: Call to undefined function feeds_importer_load_all() in /var/www/[mysite]/sites/all/modules/feeds/feeds.install on line 699
Fatal error: Call to undefined function feeds_importer_load_all() in /var/www/[mysite]/sites/all/modules/feeds/feeds.install on line 699
Drush command terminated abnormally due to an unrecoverable error. [error]
Not sure how to get out of this. This is a dev site so I'm not panicking. Yet.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | feeds-feeds_importer_load_all-2537290-9.patch | 37.82 KB | megachriz |
| #9 | feeds-feeds_importer_load_all-2537290-9-tests-only.patch | 36.02 KB | megachriz |
| #7 | feeds-feeds_importer_load_all-2537290-7.patch | 1.8 KB | twistor |
Comments
Comment #1
twistor commentedThis is probably pretty easy to fix, but I would love to reproduce it because I've heard of this before.
What version of Drush are you using?
Comment #2
memcinto commenteddrush 5.7.
Comment #3
twistor commentedComment #4
agerard commentedsame problem here... also a dev site, but would like to track it down and fix before I find it on a production site. I'm using drush 6.2.0
Comment #5
twistor commentedOne thing that can cause this problem is if feeds is disabled. I think we need to re-work the update.
Comment #6
memcinto commentedNote that I *think* originally when I updated feeds on my dev site, I thought the db updates were in a loop, because it kept repeating "Performed update: feeds_update_7211" over and over, but I didn't realize that it would need to run that multiple times for each related importer mapping. So I think I CTRL-C'd the update. Then of course it was messed up, then I tried disabling feeds, then etc. gradual but total meltdown.
Today I wiped my dev site and synced it from prod, and this time installing the new version of Feeds worked without any errors.
Comment #7
twistor commentedThis should fix the issue, but it's a terrible hack.
Comment #8
inversed commentedI haven't tried the patch but enabling Feeds before running the update for 7211 fixed the problem for me.
Comment #9
megachrizIf it is a "terrible hack", then this could at least use tests. I wrote a test that tests all database updates since 7.x-2.0-alpha7. More about that later. First into the review of the solution.
Review
I didn't test the patch manually, but the automated tests (see further) prove that it works as expected.
I wonder why the
array_reverse()is needed here. Couldn't 'feeds' and 'ctools' just be defined in reverse order or is there is a special reason for it?Tests
Based on the instructions from https://www.drupal.org/node/1429136 I wrote an update path test. The test case contains two methods:
feeds_update_7211()was done correctly and if Feeds and Ctools remain disabled after the updates are done.I chose the Feeds 7.x-2.0-alpha7 version as a starting point to test updates from because that is the oldest version I can remember to have used. And also because the one who started the issue mentioned it.
FeedsConfigurable not found error
During writing the test I kept getting the following error when running the tests:
To fix this, I modified
feeds_importer_load_all()to check for this class before it continues. Since it also checked if the functionctools_include()exists, I thought it was okay to add an extra check there. Strictly, this should be handled in an other issue, but I wanted to get going with tests for this issue.I noticed that people got this error in the past too during updating and they were told to do a registry rebuild and then continue. With this addition that would no longer be needed. See these (already closed) issues:
#1177844: Fatal error with FeedsConfigurable.inc
#2297917: Fatal error: Class 'FeedsConfigurable' not found in feeds.module on line 898
#884638: Feeds_ui ERROR: Class 'FeedsConfigurable' not found
#857246: Fatal error: Class 'FeedsConfigurable' not found
Bonus effect, side effects
As a bonus, future update functions will now also be evaluated. This is because the update path tests performs all database updates. Thus if one is added via a patch, the update path test will execute that one as well. There is also a negative side: if structural changes are made that touches things done in earlier updates (that will be the bundle setting, the file:uri target and the cache_feeds_http table), the update path test may fail. An other negative side effect is that the tests performs updates from Drupal Core and Ctools as well. Should there be a bug in the update functions in an official release from these two projects, then the update path test will fail and Feeds will appear to be broken. Should this occur, then the solution is easy: update the schema and schema version of these modules in the test and the database updates will appear to Drupal to already have ran.
Patches
Comment #12
twistor commentedWow, that's a lot of work. It's definitely a good idea.
Regarding the hack, I think trying to change importer configuration in an update is hacky in general. There's not way to ensure everything will get updated.
In this specific case, the uri should have never been added, but that's too late to change now.
Comment #13
megachrizThanks. For some reason though the tests fails on updating the bundle config for the node processor (the test passed locally). The paths of were the contrib modules are installed are also hardcoded to what the testbot uses during the tests, namely "sites/default/modules/", so that makes the test fail locally when Feeds is installed on an other location. I see I can fix that.
I put reviewing #2542416: Allow sources and targets to be defined but hidden. on my todo list. I also still am working on tests for the multilingual issue.
Comment #14
megachrizThe update
feeds_update_7211has been removed in #2542416: Allow sources and targets to be defined but hidden., which makes this issue obsolete.The tests for testing Feeds database updates are interesting to have, but that should be an other issue.
A new release is planned for next week.