PoormansCron: The Next Generation
Dave Reid - September 27, 2009 - 19:23
| Project: | Poormanscron |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Dave Reid |
| Status: | closed |
Description
Revised the entire module to be a simple backport of the current D7-style poormanscron (#331611: Add a poormanscron-like feature to core). It runs using an AJAX request instead of in hook_exit(). And it includes the issue to solve creating two separate Drupal requests on every page load by just using JavaScript to run the poormanscron request (#566494: Cron image does a full bootstrap on every page request (including cached ones)).
This version is completely D7-compatible so when the user upgrades to Drupal 7 the automatica cron setting should already be set up and ready to go.
| Attachment | Size |
|---|---|
| poormanscron-TNG-D6.patch | 10.7 KB |
#1
#2
Including tests!
#3
Including tests?! Damn skippy! Think we should wait for #566494: Cron image does a full bootstrap on every page request (including cached ones) to go through? Still not entirely sure what solution will end up going through. Either way, this is still good for an initial testing DRUPAL-6--2!
#4
Seems like #566494: Cron image does a full bootstrap on every page request (including cached ones) is going to have a lot of objectors, but I really really like that approach. Plus, I don't think we can do the
<noscript>solution since we don't have a hook_page_alter() in D6. Revised patch that passes all tests and tested locally.#5
+++ poormanscron.js 27 Sep 2009 20:27:54 -0000@@ -0,0 +1,16 @@
+ alert('Running cron!');
hm?
+++ poormanscron.module 27 Sep 2009 20:27:54 -0000@@ -7,88 +7,108 @@
+if (!defined('DRUPAL_CRON_DEFAULT_THRESHOLD')) {
+ define('DRUPAL_CRON_DEFAULT_THRESHOLD', 10800);
+}
Constants have to be in the namespace of the defining module. It doesn't matter whether this is a backport.
+++ poormanscron.module 27 Sep 2009 20:27:54 -0000@@ -7,88 +7,108 @@
+if (!defined('REQUEST_TIME')) {
+ define('REQUEST_TIME', time());
}
Please don't. Defined constants are irreversible, and modules are loaded sequentially. Use time() instead.
I'm on crack. Are you, too?
#6
Oh, yeah, I should take out the alert. But at least I can see it's working. :) Removed the DRUPAL_CRON_DEFAULT_THRESHOLD constant. Begrudgenly removing REQUEST_TIME because the performance implications of calling time() multiple times. I just reworked poormanscron_run_cron_check() to call time() once and re-use the result.
#7
+++ poormanscron.module 27 Sep 2009 21:37:06 -0000@@ -6,89 +6,103 @@
+if (!defined('REQUEST_TIME')) {
+ define('REQUEST_TIME', time());
+}
...
+function poormanscron_run_cron_check() {
...
+ $time = time();
Great - though you missed to remove the constant.
btw, time() can be called thousands of times before it starts to have any measurable performance impact.
The reason for introducing REQUEST_TIME was a different one.
This review is powered by Dreditor.
#8
Hah, thanks. :) Really appreciate the reviews.
#9
#10
6.x-2.0 branch created and now using this code. Thanks everyone!
#11
Quick question, does this embed the cron ajax/image callback for every page? or does it only inject it when cron is needed. Thinking about page caching and how this would effect it. I would prefer it was embedded in every page; maybe make it an option for people wanting it either way.
#12
The way it works is it injects a JavaScript variable timestamp for when the next cron run should happen, along with poormanscron.js which tests if the current time is greater than the 'target' timestamp, then it creates the ajax request to run cron.
#13
Cool, thanks for the info. Works how I hopped it would!
#14
Subscribing... (no I won't complain about
<noscript>, don't worry :P ... I know it is fixed, just archiving)#15
Automatically closed -- issue fixed for 2 weeks with no activity.