(Python 3) Use Redis' decode_responses=True option (#4232)

* Fix test_outdated_queries_works_scheduled_queries_tracker (use utcnow)

* Make sure Redis connection uses decoded_responses option

* Remove unused imports.

* Use Redis' decode_responses option

* Remove cases of explicit Redis decoding

* Rename helper function and make sure it doesn't apply twice.

* Don't add decode_responses to Celery Redis connection URL

redash/__init__.py

@@ -2,8 +2,6 @@
 import logging
 import os
 import sys
-import urllib.request, urllib.parse, urllib.error
-import urllib.parse
 
 import redis
 from flask_mail import Mail

redash/models/__init__.py

@@ -176,11 +176,7 @@ def paused(self):
 
     @property
     def pause_reason(self):
-        rv = redis_connection.get(self._pause_key)
-        if not rv:
-            return rv
-
-        return rv.decode()
+        return redis_connection.get(self._pause_key)
 
     def pause(self, reason=None):
         redis_connection.set(self._pause_key, reason or '')

redash/models/users.py

@@ -39,7 +39,7 @@ def sync_last_active_at():
     for user_id in user_ids:
         timestamp = redis_connection.hget(LAST_ACTIVE_KEY, user_id)
         active_at = dt_from_timestamp(timestamp)
-        user = User.query.filter(User.id == user_id.decode()).first()
+        user = User.query.filter(User.id == user_id).first()
         if user:
             user.active_at = active_at
         redis_connection.hdel(LAST_ACTIVE_KEY, user_id)

redash/settings/__init__.py

@@ -4,10 +4,13 @@
 from funcy import distinct, remove
 from flask_talisman import talisman
 
-from .helpers import fix_assets_path, array_from_string, parse_boolean, int_or_none, set_from_string
+from .helpers import fix_assets_path, array_from_string, parse_boolean, int_or_none, set_from_string, add_decode_responses_to_redis_url
 from .organization import DATE_FORMAT, TIME_FORMAT  # noqa
 
-REDIS_URL = os.environ.get('REDASH_REDIS_URL', os.environ.get('REDIS_URL', "redis://localhost:6379/0"))
+# _REDIS_URL is the unchanged REDIS_URL we get from env vars, to be used later with Celery
+_REDIS_URL = os.environ.get('REDASH_REDIS_URL', os.environ.get('REDIS_URL', "redis://localhost:6379/0"))
+# This is the one to use for Redash' own connection:
+REDIS_URL = add_decode_responses_to_redis_url(_REDIS_URL)
 PROXIES_COUNT = int(os.environ.get('REDASH_PROXIES_COUNT', "1"))
 
 STATSD_HOST = os.environ.get('REDASH_STATSD_HOST', "127.0.0.1")
@@ -24,7 +27,7 @@
 SQLALCHEMY_ECHO = False
 
 # Celery related settings
-CELERY_BROKER = os.environ.get("REDASH_CELERY_BROKER", REDIS_URL)
+CELERY_BROKER = os.environ.get("REDASH_CELERY_BROKER", _REDIS_URL)
 CELERY_RESULT_BACKEND = os.environ.get(
     "REDASH_CELERY_RESULT_BACKEND",
     os.environ.get("REDASH_CELERY_BACKEND", CELERY_BROKER))

redash/settings/helpers.py

@@ -1,4 +1,5 @@
 import os
+from urllib.parse import urlparse, urlunparse
 
 
 def fix_assets_path(path):
@@ -34,3 +35,16 @@ def int_or_none(value):
         return value
 
     return int(value)
+
+
+def add_decode_responses_to_redis_url(url):
+    """Make sure that the Redis URL includes the `decode_responses` option."""
+    parsed = urlparse(url)
+
+    query = 'decode_responses=True'
+    if parsed.query and 'decode_responses' not in parsed.query:
+        query = "{}&{}".format(parsed.query, query)
+    elif 'decode_responses' in parsed.query:
+        query = parsed.query
+
+    return urlunparse([parsed.scheme, parsed.netloc, parsed.path, parsed.params, query, parsed.fragment])

tests/test_models.py

@@ -157,7 +157,7 @@ def test_outdated_queries_works_with_ttl_based_schedule(self):
         self.assertIn(query, queries)
 
     def test_outdated_queries_works_scheduled_queries_tracker(self):
-        two_hours_ago = datetime.datetime.now() - datetime.timedelta(hours=2)
+        two_hours_ago = utcnow() - datetime.timedelta(hours=2)
         query = self.factory.create_query(schedule={'interval':'3600', 'time': None, 'until':None, 'day_of_week':None})
         query_result = self.factory.create_query_result(query=query, retrieved_at=two_hours_ago)
         query.latest_query_data = query_result