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: Running cron in a shutdown function breaks the site).

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.

CommentFileSizeAuthor
#8 589438-poormanscron-TNG-D6.patch12.83 KBdave reid
#6 589438-poormanscron-TNG-D6.patch12.98 KBdave reid
#4 poormanscron-TNG-D6.patch13.36 KBdave reid
#2 poormanscron-TNG-D6.patch13.37 KBdave reid
poormanscron-TNG-D6.patch10.7 KBdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new poormanscron-TNG-D6.patch13.37 KB

Including tests!

robloach’s picture

robloach
he/him
commented
Status: Needs review » Reviewed & tested by the community

Including tests?! Damn skippy! Think we should wait for #566494: Running cron in a shutdown function breaks the site 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!

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new poormanscron-TNG-D6.patch13.36 KB

Seems like #566494: Running cron in a shutdown function breaks the site 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.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Needs work 
+++ 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?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review
StatusFileSize
new 589438-poormanscron-TNG-D6.patch12.98 KB

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.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ 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.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 589438-poormanscron-TNG-D6.patch12.83 KB

Hah, thanks. :) Really appreciate the reviews.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Fixed

6.x-2.0 branch created and now using this code. Thanks everyone!

mikeytown2’s picture

mikeytown2 commented

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.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

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.

mikeytown2’s picture

mikeytown2 commented

Cool, thanks for the info. Works how I hopped it would!

TheRec’s picture

TheRec commented

Subscribing... (no I won't complain about <noscript>, don't worry :P ... I know it is fixed, just archiving)

Status: Fixed » Closed (fixed)

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