Merge pull request #54 from GSA/jim/090722/fixredis

Fix Redis Service Broker Connection

app/config.py

@@ -7,12 +7,10 @@ if os.environ.get('VCAP_APPLICATION'):
     # from app.cloudfoundry_config import extract_cloudfoundry_config
     # extract_cloudfoundry_config()
     vcap_services = json.loads(os.environ['VCAP_SERVICES'])
-    os.environ['REDIS_URL'] = vcap_services['aws-elasticache-redis'][0]['credentials']['uri']
+    os.environ['REDIS_URL'] = vcap_services['aws-elasticache-redis'][0]['credentials']['uri'].replace("redis", "rediss")
 
 
 class Config(object):
-    NOTIFY_ADMIN_API_CACHE_ENABLED = False  # TODO: remove when redis is fixed on remote
-
     ADMIN_CLIENT_SECRET = os.environ.get('ADMIN_CLIENT_SECRET')
     ADMIN_CLIENT_USER_NAME = os.environ.get('ADMIN_CLIENT_USERNAME')
     API_HOST_NAME = os.environ.get('API_HOST_NAME', 'localhost')
@@ -128,12 +126,8 @@ class Development(Config):
     ASSET_PATH = '/static/'
     LOGO_CDN_DOMAIN = 'static-logos.notify.tools'  # replace with our own CDN
 
-    REDIS_URL = os.environ.get('DEV_REDIS_URL', 'http://redis:6379')
-    REDIS_ENABLED = True
-
 
 class Test(Development):
-    NOTIFY_ADMIN_API_CACHE_ENABLED = True
     BASIC_AUTH_FORCE = False
     DEBUG = True
     TESTING = True
@@ -221,9 +215,6 @@ class Live(Config):
     ASSET_PATH = '/static/'  # TODO use a CDN
     LOGO_CDN_DOMAIN = 'static-logos.notifications.service.gov.uk'  # TODO use our own CDN
 
-    REDIS_URL = os.environ.get('REDIS_URL')
-    REDIS_ENABLED = True
-
     ADMIN_CLIENT_SECRET = os.environ.get('ADMIN_CLIENT_SECRET')
     ADMIN_CLIENT_USER_NAME = os.environ.get('ADMIN_CLIENT_USERNAME')
     API_HOST_NAME = os.environ.get('API_HOST_NAME')

app/main/views/platform_admin.py

@@ -3,15 +3,7 @@ import re
 from collections import OrderedDict
 from datetime import datetime
 
-from flask import (
+from flask import abort, flash, redirect, render_template, request, url_for
-    abort,
-    current_app,
-    flash,
-    redirect,
-    render_template,
-    request,
-    url_for,
-)
 from notifications_python_client.errors import HTTPError
 
 from app import (
@@ -468,13 +460,12 @@ def platform_admin_returned_letters():
 
         try:
             letter_jobs_client.submit_returned_letters(references)
-            if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+            redis_client.delete_by_pattern(
-                redis_client.delete_by_pattern(
+                'service-????????-????-????-????-????????????-returned-letters-statistics'
-                    'service-????????-????-????-????-????????????-returned-letters-statistics'
+            )
-                )
+            redis_client.delete_by_pattern(
-                redis_client.delete_by_pattern(
+                'service-????????-????-????-????-????????????-returned-letters-summary'
-                    'service-????????-????-????-????-????????????-returned-letters-summary'
+            )
-                )
         except HTTPError as error:
             if error.status_code == 400:
                 error_references = [
@@ -547,13 +538,10 @@ def clear_cache():
         groups = map(CACHE_KEYS.get, group_keys)
         patterns = list(itertools.chain(*groups))
 
-        if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        num_deleted = sum(
-            num_deleted = sum(
+            redis_client.delete_by_pattern(pattern)
-                redis_client.delete_by_pattern(pattern)
+            for pattern in patterns
-                for pattern in patterns
+        )
-            )
-        else:
-            num_deleted = 0
 
         msg = (
             f'Removed {num_deleted} objects '

app/notify_client/job_api_client.py

@@ -1,5 +1,3 @@
-from flask import current_app
-
 from app.extensions import redis_client
 from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache
 
@@ -102,12 +100,11 @@ class JobApiClient(NotifyAdminAPIClient):
         data = _attach_current_user(data)
         job = self.post(url='/service/{}/job'.format(service_id), data=data)
 
-        if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        redis_client.set(
-            redis_client.set(
+            'has_jobs-{}'.format(service_id),
-                'has_jobs-{}'.format(service_id),
+            b'true',
-                b'true',
+            ex=int(cache.DEFAULT_TTL),
-                ex=int(cache.DEFAULT_TTL),
+        )
-            )
 
         return job
 

app/notify_client/organisations_api_client.py

@@ -1,6 +1,5 @@
 from itertools import chain
 
-from flask import current_app
 from notifications_python_client.errors import HTTPError
 
 from app.extensions import redis_client
@@ -54,12 +53,11 @@ class OrganisationsClient(NotifyAdminAPIClient):
     def update_organisation(self, org_id, cached_service_ids=None, **kwargs):
         api_response = self.post(url="/organisations/{}".format(org_id), data=kwargs)
 
-        if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        if cached_service_ids:
-            if cached_service_ids:
+            redis_client.delete(*map('service-{}'.format, cached_service_ids))
-                redis_client.delete(*map('service-{}'.format, cached_service_ids))
 
-            if 'name' in kwargs:
+        if 'name' in kwargs:
-                redis_client.delete(f'organisation-{org_id}-name')
+            redis_client.delete(f'organisation-{org_id}-name')
 
         return api_response
 

app/notify_client/service_api_client.py

@@ -1,6 +1,5 @@
 from datetime import datetime
 
-from flask import current_app
 from notifications_utils.clients.redis import daily_limit_cache_key
 
 from app.extensions import redis_client
@@ -139,7 +138,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
     # @cache.delete('service-{service_id}-templates')
     # @cache.delete_by_pattern('service-{service_id}-template-*')
     def archive_service(self, service_id, cached_service_user_ids):
-        if cached_service_user_ids and current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        if cached_service_user_ids:
             redis_client.delete(*map('user-{}'.format, cached_service_user_ids))
         return self.post('/service/{}/archive'.format(service_id), data=None)
 
@@ -596,7 +595,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
         broadcast channel is one of "operator", "test", "severe", "government"
         provider_restriction is one of "all", "three", "o2", "vodafone", "ee"
         """
-        if cached_service_user_ids and current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        if cached_service_user_ids:
             redis_client.delete(*map('user-{}'.format, cached_service_user_ids))
 
         data = {
@@ -610,10 +609,8 @@ class ServiceAPIClient(NotifyAdminAPIClient):
     def get_notification_count(self, service_id):
         # if cache is not set, or not enabled, return 0
 
-        if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        count = redis_client.get(daily_limit_cache_key(service_id)) or 0
-            count = redis_client.get(daily_limit_cache_key(service_id)) or 0
+
-        else:
-            count = 0
         return int(count)
 
 

app/notify_client/status_api_client.py

@@ -1,4 +1,4 @@
-from app.notify_client import NotifyAdminAPIClient
+from app.notify_client import NotifyAdminAPIClient, cache
 
 
 class StatusApiClient(NotifyAdminAPIClient):
@@ -10,5 +10,9 @@ class StatusApiClient(NotifyAdminAPIClient):
     def get_count_of_live_services_and_organisations(self):
         return self.get(url='/_status/live-service-and-organisation-counts')
 
+    @cache.set('live-service-and-organisation-counts', ttl_in_seconds=3600)
+    def get_count_of_live_services_and_organisations_cached(self):
+        return self.get(url='/_status/live-service-and-organisation-counts')
+
 
 status_api_client = StatusApiClient()

app/notify_client/template_folder_api_client.py

@@ -1,5 +1,3 @@
-from flask import current_app
-
 from app.extensions import redis_client
 from app.notify_client import NotifyAdminAPIClient
 
@@ -50,7 +48,7 @@ class TemplateFolderAPIClient(NotifyAdminAPIClient):
             'folders': list(folder_ids),
         })
 
-        if template_ids and current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']:
+        if template_ids:
             redis_client.delete(*(f'service-{service_id}-template-{id}-version-None' for id in template_ids))
 
     # @cache.delete('service-{service_id}-template-folders')

app/status/views/healthcheck.py

@@ -1,7 +1,12 @@
+import time
+import traceback
+
 from flask import current_app, jsonify, request
 from notifications_python_client.errors import HTTPError
+from redis import RedisError
 
 from app import status_api_client, version
+from app.extensions import redis_client
 from app.status import status
 
 
@@ -12,11 +17,49 @@ def show_status():
     else:
         try:
             api_status = status_api_client.get_status()
-        except HTTPError as e:
+        except HTTPError as err:
             current_app.logger.exception("API failed to respond")
-            return jsonify(status="error", message=str(e.message)), 500
+            return jsonify(status="error", message=str(err.message)), 500
         return jsonify(
             status="ok",
             api=api_status,
             git_commit=version.__git_commit__,
             build_time=version.__time__), 200
+
+
+@status.route('/_status/redis', methods=['GET'])
+def show_redis_status():
+    try:
+        try:
+            api_status = {"error": "unexpected error"}
+            redis_uri = current_app.config['REDIS_URL']
+            current_app.logger.info(f"config['REDIS_URL']: {redis_uri}")
+            redis_enabled = current_app.config['REDIS_ENABLED']
+            current_app.logger.info(f"config['REDIS_ENABLED']: {redis_enabled}")
+            if redis_enabled:
+                current_app.logger.info("config['REDIS_ENABLED'] evaluates as True")
+
+            try:
+                now = time.time()
+                redis_client.set('mytestkey', now)
+                val = redis_client.get('mytestkey')
+                current_app.logger.info(f"Retrieved value from redis for mytestkey is: {val}")
+            except RedisError as err:
+                current_app.logger.exception(f"Redis service failed to respond with {err}")
+
+            api_status = status_api_client.get_count_of_live_services_and_organisations_cached()
+        except HTTPError as err:
+            current_app.logger.exception("API failed to respond")
+            return jsonify(status="error", message=str(err.message)), 500
+        return jsonify(
+            status="ok",
+            api=api_status,
+            git_commit=version.__git_commit__,
+            build_time=version.__time__), 200
+    except Exception as err:
+        current_app.logger.exception(F"Unhandled exception: {err} - {traceback.format_exc()}")
+        return jsonify(
+            status=f"error: {err}",
+            api=api_status,
+            git_commit=version.__git_commit__,
+            build_time=version.__time__), 500

devcontainer-admin/.devcontainer.json

@@ -14,10 +14,13 @@
         "python.linting.enabled": true,
         "python.linting.pylintEnabled": true,
         "python.linting.flake8Enabled": true,
-        "python.pythonPath": "/usr/local/bin/python",
+        "python.defaultInterpreterPath": "/usr/bin/python3",
         "python.linting.flake8Path": "/usr/local/py-utils/bin/flake8",
         "python.linting.pylintPath": "/usr/local/py-utils/bin/pylint",
-        "python.formatting.autopep8Path": "/usr/local/py-utils/bin/autopep8"
+        "python.formatting.autopep8Path": "/usr/local/py-utils/bin/autopep8",
+        "python.analysis.extraPaths": [
+            "/home/vscode/.local/lib/python3.9/site-packages"
+        ]
     },
     "extensions": [
         "donjayamanne.python-extension-pack",

tests/app/test_config.py

@@ -35,7 +35,7 @@ def test_load_cloudfoundry_config_if_available(reload_config):
     os.environ['VCAP_SERVICES'] = json.dumps({
         'aws-elasticache-redis': [{
             'credentials': {
-                'uri': 'redis uri'
+                'uri': 'redis://xxx:6379'
             }
         }],
     })
@@ -44,8 +44,8 @@ def test_load_cloudfoundry_config_if_available(reload_config):
         # reload config so that its module level code (ie: all of it) is re-instantiated
         importlib.reload(config)
 
-    assert os.environ['REDIS_URL'] == 'redis uri'
+    assert os.environ['REDIS_URL'] == 'rediss://xxx:6379'
-    assert config.Config.REDIS_URL == 'redis uri'
+    assert config.Config.REDIS_URL == 'rediss://xxx:6379'
 
 
 def test_load_config_if_cloudfoundry_not_available(reload_config):

tests/app/test_navigation.py

@@ -426,7 +426,7 @@ def test_all_endpoints_are_covered(navigation_instance):
     covered_endpoints = (
         navigation_instance.endpoints_with_navigation +
         EXCLUDED_ENDPOINTS +
-        ('static', 'status.show_status', 'metrics')
+        ('static', 'status.show_status', 'status.show_redis_status', 'metrics')
     )
 
     for endpoint in all_endpoints: