Drupal 7 version supports SASL authentication D8 version needs to provide parity.

Comments

Rok Žlender created an issue. See original summary.

Rok Žlender’s picture

I am working on this and should have a patch shortly.

Rok Žlender’s picture

StatusFileSize
new2.48 KB

I tested the patch with memcached server running with and without SASL and was able to connect.

Rok Žlender’s picture

Status: Active » Needs review
damiankloip’s picture

Status: Needs review » Needs work
  1. +++ b/README.txt
    @@ -225,3 +225,28 @@ Other options you could experiment with:
    +  $conf['memcache_options'] = array(
    +    Memcached::OPT_BINARY_PROTOCOL => TRUE,
    +  );
    

    The format of these is incorrect I think? These are all in settings now, like you have above for the sasl credentials.

    I think this should be:

    $settings['memcache']['options'] = [
      \Memcached::OPT_BINARY_PROTOCOL => TRUE,
    ];
    
  2. +++ b/README.txt
    @@ -225,3 +225,28 @@ Other options you could experiment with:
    +  $conf['memcache_sasl_username'] = 'yourSASLUsername';
    +  $conf['memcache_sasl_password'] = 'yourSASLPassword';
    

    These are old D7 settings are duplicates of the correct sasl array above?

  3. +++ b/src/DrupalMemcacheFactory.php
    @@ -49,6 +49,11 @@ class DrupalMemcacheFactory {
    +  protected $memcacheSasl = array();
    

    I think just @sasl is an ok name. Although, looking at this property, it doesn't seem to be used in the patch?

Rok Žlender’s picture

Status: Needs work » Needs review
StatusFileSize
new1.99 KB
new636 bytes

Thanks for the review should be all updated in the new patch.

damiankloip’s picture

Status: Needs review » Fixed

This looks good now - thanks. We could check that username and password are explicitly set to avoid warnings but I think this is fine. Committed and pushed.

Status: Fixed » Closed (fixed)

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