Hi,

Here is a patch against latest 7.x-2.x-dev for environments where Redis requires a password for authentication.

The patch implements a new password field on the Radioactivity admin interface /admin/structure/radioactivity/settings
and logic around it to provide Redis authentication.

The configuration assistant will dump accordingly another line like:
define('VAR_RADIOACTIVITY_REDIS_PASS', 'your_password');
in the admin interface which has to be added to the radioactivity-bootstrap.cfg.inc file.

CommentFileSizeAuthor
#5 radioactivity-provide_redis_password_authentication-2799331-5.patch3.58 KBkrisahil
#4 radioactivity-provide_redis_password_authentication-2799331-4.patch3.07 KBkrisahil
#2 2799331-provide_redis_password_authentication.patch2.96 KBmihaic
redis_password.png59.08 KBmihaic
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mihaic created an issue. See original summary.

mihaic’s picture

mihaic CreditAttribution: mihaic as a volunteer commented
FileSize
2799331-provide_redis_password_authentication.patch2.96 KB
samt2497’s picture

samt2497 CreditAttribution: samt2497 at Four Kitchens commented

Patch #2 works for me

krisahil’s picture

krisahil CreditAttribution: krisahil at GenomeWeb commented
Status: Active » Needs review
FileSize
radioactivity-provide_redis_password_authentication-2799331-4.patch3.07 KB

I re-rolled this patch against 7.x-2.x. This new patch has been working for large site well.

krisahil’s picture

krisahil CreditAttribution: krisahil at GenomeWeb commented
FileSize
radioactivity-provide_redis_password_authentication-2799331-5.patch3.58 KB

Updated patch fixes a bug where the Redis password does not get defined during full Drupal bootstrap. The use case I saw this in was during radioactivity's cron job, which resulted in data from Redis never getting pulled into Drupal's database.

Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan commented

@krisahilm, thanks for the updated patch. I have no experience with Radioactivity on Redis myself.
Can you provide a list of scenario's for which you have tested this patch with.
And, for which scenario's have you not tested the patch?

Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan at LimoenGroen commented
Status: Needs review » Needs work 
+++ b/includes/RadioactivityRedisIncidentStorage.inc
@@ -17,10 +17,13 @@ class RadioactivityRedisIncidentStorage extends RadioactivityIncidentStorage {
+      $redis->auth($pass);
     } else {
       $redis->connect(VAR_RADIOACTIVITY_REDIS_HOST);
+      $redis->auth($pass);
     }

Does this work if redis does not require authentication, or should $redis->auth() be called conditionally? I don't want to break any existing installations.

No further comments. I will commit once the above remark has been resolved.