Dynamic_cache should be bypassed in certain admin contexts (drush, cron, etc.)

I wonder if this is at all related to #1515406: Ensure dynamic_cache is only implimented for index.php? I'm not sure about the specifics of what Drush might be doing that breaks things, but I suppose that if any drush code invokes a full Drush bootstrap, having this module enabled could certainly cause problems. This is basically the issue I encountered when running cron.php (dynamic cache fired, hijacking the bootstrap that happens before drupal_cron_run(), and cron never ran).

Would limiting this module to running only under index.php (see patch in #1515406: Ensure dynamic_cache is only implimented for index.php) help here?

Yeah, that makes sense. I wonder if there is another way to test if we are inside a drush context and always bypass dynamic_cache if so? I'm not sure what that would be, especially if drush is just running a normal index.php request.

I suppose there could always be some form of new "exception" popping up where dynamic_cache hijacks the bootstrap when it shouldn't. I'm not sure if finding and checking for all the possible exceptions makes more sense than just simply using a flag other than $GLOBALS['conf']['cache'] (as you suggested). I guess the big downside of using another flag that is that it would mean that dynamic_cache is slightly less of a "drop in" solution (as any modules that need to conditionally disable the cache in hook_boot cannot use a native Drupal variable to do it).

Anyway if it's decided to change the flag, then the point about "setting $GLOBALS['conf']['cache'] after having checked for the flag" is quite important. Typically we would not want drupal_page_footer() to write a page that's served up without the cache, due to dynamic_cache, into the DB cache. At least I don't think so.

So I can indeed duplicate this. It looks like this module, as-is, can simply break drush. The problem appears to be the simple fact that drush directly calls drupal_bootstrap(), which can then trigger dynamic_cache. I think I might have a way around this though, and will try to post a patch.

Yeah, as noted in #3, it's hard to say whether it makes more sense to id and check for all the exceptions when dynamic_cache should be bypassed, or come up with some totally different approach. For the moment it looks like at least the following cases should lead to a bypass:

1. Bootstrap is called outside of index.php (e.g. cron.php)
2. Bootstrap is called within drush

The attached patch attempts to accommodate for both of these. The first is detailed more in #1515406: Ensure dynamic_cache is only implimented for index.php and the second is addressed by looking for a constant that's defined in drushs's own bootstrap.inc (where drupal_bootstrap() is called).

It would be interesting to know if this works for others, or if other exception cases can be defined.

Just trying to generalize the title a bit more, and also marking #1515406: Ensure dynamic_cache is only implimented for index.php as a duplicate of this.

Nice, even cleaner. I might up the ante a bit by suggesting the we also add this right before the final if ($GLOBALS['conf']['cache'] == CACHE_DISABLED) check:

  // Let other modules step-in at the last min to bypass. This is really only 
  // useful in a situation where $GLOBALS['conf']['cache'] needs to be set to 
  // CACHE_DISABLED but another module still wants to force a normal bootstrap.
  // NOTE: any implementing modules MUST be set 'bootable'.
  if (array_search(TRUE, module_invoke_all('bypass_dynamic_cache')) !== FALSE) {
    return FALSE;
  }

This would allow the bypass option to be even further configurable. I'm not sure if this is just over-complicating things though. I'm happy to roll that into another patch atop #8 if it makes sense.

Fair enough, I often just lean toward the side of 'baked-in flexibility', but that may not apply here. Let's leave things are they are in #8 and hope to get an added review, or simply a commit. This module is pending review for full project status and, IMHO, this is the only bug that really needs to be addressed before that point (along with some simple coder checks, etc.)...

The patch in #8 has been included in the RC1 branch.

Btw, thanks for the work on this guys.

Are you able to test that the current RC1 release on http://drupal.org/project/dynamic_cache fixes this issue (works with drush, etc)? Would like to verify before marking this issue as fixed.

Hi Brian,

I've been using the patch (and now the rc1 with the patch included) for some time now in production, and things look good.

Btw, with regards to the issue queue status, once you feel that a fix/patch has been tested enough to commit to dev, you can typically mark the issue as "fixed" right then. After that, drupal.org will automatically toggle it to "fixed (closed)" after 2 weeks of inactivity. This gives some time for people to re-visit the issue after using dev if needed (see http://drupal.org/node/156119).

Thanks for getting things cleaned up so this project could be officially approved.

Great, thanks. I just didn't really want to mark this as "fixed" until someone had at least tried the code out with ie, Drush, as it's not something I'm familiar enough with to test properly myself.

With this issue verified, I think the 6.x branch can move to a 1.0 release.

Btw, I left the exact same tests in place for the 7.x branch, do you happen to know offhand if testing for the defined Drush constant should work the same way in 7.x?

Humm, I think the same DRUSH_BOOTSTRAP_DRUPAL_FULL constant should be defined no matter what version of Drupal Drush is working with. The same version of Drush is used for different versions of Drupal and it looks like they all ultimately call drush.php. Inside drush.php is a require_once for /includes/environment.inc, and in there is define('DRUSH_BOOTSTRAP_DRUPAL_FULL', 5).

So I think that whenever Drush runs, for any version of Drupal, DRUSH_BOOTSTRAP_DRUPAL_FULL will be defined.

I'm not super familiar with Drush either, but my best guess is that the same constant check would work for any Drupal version of dymanic_cache.