I'm adding some revised documentation for DrupalCacheInterface. The instructions specify to set a variable called "cache_page" to override the cache_page bin.

This is incorrect because _cache_get_object() looks for a variable named "cache_class_" . $bin:

function _cache_get_object($bin) {
  // We do not use drupal_static() here because we do not want to change the
  // storage of a cache bin mid-request.
  static $cache_objects;
  if (!isset($cache_objects[$bin])) {
    $class = variable_get('cache_class_' . $bin);
    if (!isset($class)) {
      $class = variable_get('cache_default_class', 'DrupalDatabaseCache');
    }
    $cache_objects[$bin] = new $class($bin);
  }
  return $cache_objects[$bin];
}

The revised documentation mentions the correct naming convention.

I also added a block about creating a fully custom cache bin. I think that this will be as common as overriding Drupal's cache bins. The extra documentation will help.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Good catch!

I think this is a good addition to the documentation. Thanks!

jhodgdon’s picture

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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

Kevin Hankens’s picture

Status: Closed (fixed) » Needs review
FileSize
1.21 KB

So, it looks like the previously committed patch has been overwritten, though I don't see anything in the log that suggests it was ever committed. ???

Anyway, I think that this is an important patch, because the documentation is incorrect.

Thx!

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.24 KB

You have to roll patches starting from Drupal head. Please read
http://drupal.org/patch/create

But you are correct, it doesn't look like the patch in the original issue report was ever committed (which is the same as the one in #5 except that it is rolled correctly). So attaching that patch again to send to the test bot.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Kevin Hankens’s picture

Oops, yeah, that was my bad. Thx for resubmitting!

Status: Fixed » Closed (fixed)

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