Expected behavior would be for update status to be run during cron without having to check manually (via admin/reports/status/check) first. Once update status is checked manually, cron update checks work as expected.
This is really confusing, because the status page for update status indicates you can run cron to get update information during the first run, which you can't:
see http://skitch.com/duellj/d6hk6/status-report-drupal-7
As far as I can tell, here is what's going on:
In update_cron:
function update_cron() {
$frequency = variable_get('update_check_frequency', 1);
$interval = 60 * 60 * 24 * $frequency;
if ((REQUEST_TIME - variable_get('update_last_check', 0)) > $interval) {
// If the configured update interval has elapsed, we want to invalidate
// the cached data for all projects, attempt to re-fetch, and trigger any
// configured notifications about the new status.
update_refresh();
_update_cron_notify();
}
else {
// Otherwise, see if any individual projects are now stale or still
// missing data, and if so, try to fetch the data.
update_get_available(TRUE);
}
}
Until the variable update_last_check is set, the second conditional block will never run (which does the actual checking of stale projects). update_last_check is only set in _update_create_fetch_task(), which is called from update_fetch_data(), which is only called through update_get_available(). So update_last_check is never set during cron, only during manual checks.
So would this be as simple as calling variable_set('update_last_check', time()); in the first conditional block?
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | StatusReportAfterOutsideCron.png | 58.77 KB | mark trapp |
| #8 | 962802-7.step-4.png | 106.29 KB | dww |
| #6 | cron_status.png | 88.54 KB | duellj |
| #4 | 962802-4.step1_.png | 84.94 KB | dww |
| #4 | 962802-4.step2_.png | 96.25 KB | dww |
Comments
Comment #1
duellj commentedAdding in variable_set('update_last_check', time()) to update_cron works. Attached patch.
Comment #2
duellj commentedMaking title a bit more descriptive
Comment #3
mark trappTagging. Looks good to me, but there might be a good reason that it's like that I'm not aware of.
Comment #4
dwwI just tried this on a fresh install of D7 using the latest in CVS HEAD and couldn't reproduce the problem:
- Totally clean install, using the standard profile.
- Visited admin/reports/status, it said there was no available update info.
- Clicked on the link from "You can run cron manually." under "Cron maintenance tasks".
- Landed on admin/reports/status again, which said it had status info (and was confused about my install since I checked out from CVS but didn't enable cvs_deploy yet).
- Clicked on the "available updates" link from there, landed on admin/reports/updates and it has info and says it last checked 39 seconds ago.
Attached screenies of what I saw.
I don't see a bug here. Can anyone reproduce and/or explain how my testing was invalid?
Thanks,
-Derek
Comment #5
dwwp.s. I don't agree with the patch here, nor the assessment of the code flow. We set update_last_check whenever we actually check. update_cron() should be triggering a check when needed. If 'update_last_check' isn't set, we should get a 0, which should definitely trigger the "we need to fetch" case. I don't know what duellj was seeing... ;)
Comment #6
duellj commentedThanks for the input, dww. Here's how the bug is reproduced (using a fresh install of D7 on HEAD):
So the problem is that as long as cron is run outside of the site (via cron.php), update status will never run (unless admin/reports/status/run-cron or admin/reports/updates/check is executed).
The problem is that during the cron.php run, since update_last_check defaults to 0 and isn't set until update_get_available() is run, the second conditional block is never run, thus update_get_available() is never run. The "we need to fetch" case is always run, and we never get to the actual update status run.
Comment #7
dwwInteresting, thanks for the info. I don't have time right now to look more closely but it sounds reproducible, and like a major bug. I still don't think the solution from #1 is right. I'd like to find a better way to solve this. But first, I'd need to follow the code during a cron.php run and see what comes up.
Comment #8
dwwI still can't reproduce.
At step #4 from the instructions in comment 6, the status report says it fetched (although again, on my site, it's confused since I deployed from CVS but haven't enabled cvs_deploy yet by the time I'm hitting this page). See attached.
Can anyone *else* reproduce this?
Comment #9
duellj commentedOdd. I'd be interested in seeing if anyone else can reproduce this too, since I'm still seeing it on my site (using HEAD deployed from CVS on a brand new install).
Comment #10
mark trappI can reproduce this with latest HEAD from CVS following the instructions in #6 (I've been able to reproduce it since tagging in #3).
Comment #11
bfroehle commentedThere is some code flow problem here because you can run cron on a fresh install until you are blue in the face and
update_last_checknever gets set. There's no difference between how you can run cron, so that's just a red herring.In update_cron, update_refresh only creates the queue and all of the jobs. We then actually need to call update_fetch_data() or similar to do the actual data fetching... Not sure about code flow here, as we only allot 5 seconds to fetch the data and we probably don't want to send the email in _update_cron_notify until all of the data is fetched....
Comment #12
bfroehle commentedMarking this as a duplicate of #647964: Running cron does not check for available updates which is the underlying problem.
Comment #13
bfroehle commentedI've posted a potential patch in #647964-29: Running cron does not check for available updates and would appreciate some manual testing of it, if possible. Thanks!