Support from Acquia helps fund testing for Drupal Acquia logo

Comments

totosarg’s picture

Patch for reducing the severity of memcache version mismatch is attached.

totosarg’s picture

Status: Active » Needs review
Jeremy’s picture

This affects too many valid errors. We should only make an out-dated 'memcache PECL extension' a warning instead of an error, because currently there's no "stable" recommended release.

The following should still be considered errors, not warnings:

  • $errors[] = $t('Required PHP extension not found. Install the <a href="http://php.net/manual/en/book.memcache.php">memcache</a> (recommended) or <a href="http://php.net/manual/en/book.memcached.php">memcached</a> extension.');
  • $errors[] = $t('Failed to load required file %dmemcache.', array('%dmemcache' => drupal_get_path('module', 'memcache') . '/' . 'dmemcache.inc'));
  • $errors[] = $t('Failed to connect to memcached server instance at %server.', array('%server' => $server));
  • $errors[] = $t('Failed to store to then retrieve data from memcache.');
  • $errors[] = $t('Unexpected failure when testing memcache configuration.');

It's the following error we can turn into a warning, but only in the case of the 'memcache' PECL extension, not in the case of the 'memcached' PECL extension:

  • $errors[] = $t('PECL !extension version %version is unsupported. Please update to %recommended or newer.', array(
Jeremy’s picture

Actually, we can make all version checks a warning -- no need to differentiate between PECL memcache and memcached.

totosarg’s picture

Done. Also, the code in the patch allows you to populate warnings down the road fairly easily.

totosarg’s picture

Last iteration, per your comment, removed duplicate error message code.

totosarg’s picture

further reduced message duplication.

totosarg’s picture

Jeremy’s picture

Status: Needs review » Needs work
FileSize
3.86 KB

This is getting closer. I've done a little cleanup to follow Drupal.org coding standards ( https://drupal.org/coding-standards ) and made sure all text is translatable. Attaching an updated patch.

However, you've got an order of presidence issue -- if there's a warning it must be displayed, whereas we don't see errors if there's a warning. I killed memcache and had an outdated PECL, and all I get is a warning that PECL is outdated, no error that memcache isn't running.

totosarg’s picture

Jeremy, i made the changes per our discussion. Attached in the patch.

Jeremy’s picture

Status: Needs review » Fixed
totosarg’s picture

Thanks Jeremy! It was a pleasure to meet you, and i appreciate all the feedback :)

  • Commit c7c0120 on 7.x-1.x authored by totosarg, committed by Jeremy:
    Issue #2278021 by totosarg, Jeremy: lighten memcache version warning.
    

Status: Fixed » Closed (fixed)

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