#2069 - Reduced Write Instance Queries (#2071)

.talismanrc

@@ -47,4 +47,6 @@ fileignoreconfig:
   checksum: 4e15e63d349635131173ffdd7aebcd547621db08de877ef926d3a41fde72d065
 - filename: tests/app/v2/notifications/test_post_notifications.py
   checksum: 3181930a13e3679bb2f17eaa3f383512eb9caf4ed5d5e14496ca4193c6083965
+- filename: app/va/va_profile/va_profile_client.py
+  checksum: fe634f26f7dc3874f4afcfd1ba3f03bae380b53befe973a752c7347097a88701
 version: "1.0"

README.md

@@ -96,8 +96,9 @@ To support running locally, the repository includes a default `app/version.py` f
 Running `flask db migrate` on the container ci_app_1 errors because the files in the migrations folder are read-only.  Follow this procedure to create a database migration using Flask:
 
 1. Ensure all containers are stopped and that the notification_api image has been built
-2. Run `docker compose -f ci/docker-compose-local-migrate.yml up`.  This creates the container ci_app_migrate with your local notification-api directory mounted in read-write mode.  The container runs `flask db migrate` and exits.
-3. Press Ctrl-C to stop the containers, and identify the new file in `migrations/versions/`.
+2. Run migrations at least once e.g. `docker compose -f ci/docker-compose-local.yml up` and stop any running containers.
+3. Run `docker compose -f ci/docker-compose-local-migrate.yml up`.  This creates the container ci_app_migrate with your local notification-api directory mounted in read-write mode.  The container runs `flask db migrate` and exits.
+4. Press Ctrl-C to stop the containers, and identify the new file in `migrations/versions/`.
 
 ### Unit testing
 See the [tests README.md](tests/README.md) for information.

app/__init__.py

@@ -43,6 +43,7 @@
 
 DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%fZ'
 DATE_FORMAT = '%Y-%m-%d'
+HTTP_TIMEOUT = (3.05, 1) if os.getenv('NOTIFY_ENVIRONMENT') in ('production', 'staging') else (30, 30)
 
 load_dotenv()
 

app/callback/webhook_callback_strategy.py

@@ -8,7 +8,7 @@
 from requests.api import request
 from requests.exceptions import HTTPError, RequestException
 
-from app import statsd_client
+from app import statsd_client, HTTP_TIMEOUT
 from app.callback.service_callback_strategy_interface import ServiceCallbackStrategyInterface
 from app.celery.exceptions import NonRetryableException, RetryableException
 from app.dao.api_key_dao import get_unsigned_secret
@@ -32,7 +32,7 @@ def send_callback(
                     'Content-Type': 'application/json',
                     'Authorization': 'Bearer {}'.format(callback.bearer_token),
                 },
-                timeout=(3.05, 1),
+                timeout=HTTP_TIMEOUT,
             )
             current_app.logger.info('Callback sent to %s, response %d, %s', callback.url, response.status_code, tags)
             response.raise_for_status()

app/celery/process_ga4_measurement_tasks.py

@@ -4,8 +4,9 @@
 
 from flask import current_app
 
-from app import db, notify_celery
+from app import notify_celery
 from app.celery.exceptions import AutoRetryException
+from app.dao.dao_utils import get_reader_session
 from app.models import Notification, NotificationHistory, TemplateHistory
 
 
@@ -54,24 +55,25 @@ def post_to_ga4(notification_id: str, event_name, event_source, event_medium) ->
         current_app.logger.error('GA4_MEASUREMENT_ID is not set')
         return False
 
-    # Retrieve the notification from the database.  It might have moved to history.
-    notification = db.session.get(Notification, notification_id)
-    if notification is None:
-        notification = db.session.get(NotificationHistory, notification_id)
+    with get_reader_session() as session:
+        # Retrieve the notification from the database.  It might have moved to history.
+        notification = session.get(Notification, notification_id)
         if notification is None:
-            current_app.logger.warning('GA4: Notification %s not found', notification_id)
-            return False
+            notification = session.get(NotificationHistory, notification_id)
+            if notification is None:
+                current_app.logger.warning('GA4: Notification %s not found', notification_id)
+                return False
+            else:
+                # The notification is a NotificationHistory instance.
+                template_id = notification.template_id
+                template_name = session.get(TemplateHistory, (template_id, notification.template_version)).name
         else:
-            # The notification is a NotificationHistory instance.
-            template_id = notification.template_id
-            template_name = db.session.get(TemplateHistory, (template_id, notification.template_version)).name
-    else:
-        # The notification is a Notification instance.
-        template_id = notification.template.id
-        template_name = notification.template.name
+            # The notification is a Notification instance.
+            template_id = notification.template.id
+            template_name = notification.template.name
 
-    service_id = notification.service_id
-    service_name = notification.service.name
+        service_id = notification.service_id
+        service_name = notification.service.name
 
     url_str = current_app.config.get('GA4_URL', '')
     url_params_dict = {

app/celery/process_ses_receipts_tasks.py

@@ -14,7 +14,8 @@
 from sqlalchemy.orm.exc import NoResultFound
 import enum
 import requests
-from app import DATETIME_FORMAT, notify_celery, statsd_client, va_profile_client
+
+from app import DATETIME_FORMAT, HTTP_TIMEOUT, notify_celery, statsd_client, va_profile_client
 from app.celery.exceptions import AutoRetryException
 from app.celery.service_callback_tasks import publish_complaint
 from app.config import QueueNames
@@ -74,7 +75,7 @@ def get_certificate(url):
     res = certificate_cache.get(url)
     if res is not None:
         return res
-    res = requests.get(url, timeout=(3.05, 1)).content
+    res = requests.get(url, timeout=HTTP_TIMEOUT).content
     certificate_cache.set(url, res, timeout=60 * 60)  # 60 minutes
     return res
 
@@ -106,7 +107,7 @@ def sns_callback_handler():
     if message.get('Type') == 'SubscriptionConfirmation':
         url = message.get('SubscribeURL')
         try:
-            response = requests.get(url, timeout=(3.05, 1))
+            response = requests.get(url, timeout=HTTP_TIMEOUT)
             response.raise_for_status()
         except requests.RequestException as e:
             current_app.logger.warning('Response: %s', response.text)
@@ -140,7 +141,7 @@ def sns_smtp_callback_handler():
     if message.get('Type') == 'SubscriptionConfirmation':
         url = message.get('SubscribeURL')
         try:
-            response = requests.get(url, timeout=(3.05, 1))
+            response = requests.get(url, timeout=HTTP_TIMEOUT)
             response.raise_for_status()
         except requests.RequestException as e:
             current_app.logger.warning('Response: %s', response.text)

app/celery/service_callback_tasks.py

@@ -6,7 +6,7 @@
 from requests.exceptions import Timeout, RequestException
 from notifications_utils.statsd_decorators import statsd
 
-from app import notify_celery, encryption, statsd_client, DATETIME_FORMAT
+from app import notify_celery, encryption, statsd_client, DATETIME_FORMAT, HTTP_TIMEOUT
 from app.callback.webhook_callback_strategy import generate_callback_signature
 from app.celery.exceptions import AutoRetryException, NonRetryableException, RetryableException
 from app.config import QueueNames
@@ -394,7 +394,7 @@ def send_delivery_status_from_notification(
                 'Content-Type': 'application/json',
                 'x-enp-signature': callback_signature,
             },
-            timeout=(3.05, 1),
+            timeout=HTTP_TIMEOUT,
         )
         response.raise_for_status()
     except Timeout as e:

app/clients/email/govdelivery_client.py

@@ -1,4 +1,5 @@
 import requests
+
 from app.clients.email import EmailClient, EmailClientException
 from app.models import (
     NOTIFICATION_CANCELLED,
@@ -36,6 +37,9 @@ def init_app(
         *args,
         **kwargs,
     ):
+        from app import HTTP_TIMEOUT  # Circular import
+
+        self.timeout = HTTP_TIMEOUT
         self.name = 'govdelivery'
         self.token = token
         self.statsd_client = statsd_client
@@ -75,7 +79,7 @@ def send_email(
 
             start_time = monotonic()
             response = requests.post(
-                self.govdelivery_url, json=payload, headers={'X-AUTH-TOKEN': self.token}, timeout=(3.05, 1)
+                self.govdelivery_url, json=payload, headers={'X-AUTH-TOKEN': self.token}, timeout=self.timeout
             )
             response.raise_for_status()
 

app/clients/performance_platform/performance_platform_client.py

@@ -14,6 +14,9 @@ def init_app(
         self,
         app,
     ):
+        from app import HTTP_TIMEOUT  # Circular import
+
+        self.timeout = HTTP_TIMEOUT
         self._active = app.config.get('PERFORMANCE_PLATFORM_ENABLED')
         if self.active:
             self.performance_platform_url = app.config.get('PERFORMANCE_PLATFORM_URL')
@@ -27,7 +30,7 @@ def send_stats_to_performance_platform(
             bearer_token = self.performance_platform_endpoints[payload['dataType']]
             headers = {'Content-Type': 'application/json', 'Authorization': 'Bearer {}'.format(bearer_token)}
             resp = requests.post(
-                self.performance_platform_url + payload['dataType'], json=payload, headers=headers, timeout=(3.05, 1)
+                self.performance_platform_url + payload['dataType'], json=payload, headers=headers, timeout=self.timeout
             )
 
             if resp.status_code == 200:

app/cronitor.py

@@ -4,6 +4,8 @@
 
 
 def cronitor(task_name):
+    from app import HTTP_TIMEOUT  # Circular import
+
     # check if task_name is in config
     def decorator(func):
         def ping_cronitor(command):
@@ -25,7 +27,7 @@ def ping_cronitor(command):
                     params={
                         'host': current_app.config['API_HOST_NAME'],
                     },
-                    timeout=(3.05, 1),
+                    timeout=HTTP_TIMEOUT,
                 )
                 resp.raise_for_status()
             except requests.RequestException:

app/notifications/utils.py

@@ -3,13 +3,15 @@
 
 
 def confirm_subscription(confirmation_request):
+    from app import HTTP_TIMEOUT  # Circular import
+
     url = confirmation_request.get('SubscribeURL')
     if not url:
         current_app.logger.warning('SubscribeURL does not exist or empty.')
         return
 
     try:
-        response = requests.get(url, timeout=(3.05, 1))
+        response = requests.get(url, timeout=HTTP_TIMEOUT)
         response.raise_for_status()
     except requests.RequestException:
         current_app.logger.exception('Response: %s', response.text)

app/notifications/validators.py

@@ -1,7 +1,7 @@
 import base64
 import binascii
 
-from sqlalchemy.orm.exc import NoResultFound
+from cachetools import TTLCache, cached
 from flask import current_app
 from notifications_utils import SMS_CHAR_COUNT_LIMIT
 from notifications_utils.recipients import (
@@ -10,6 +10,7 @@
     get_international_phone_info,
 )
 from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_cache_key
+from sqlalchemy.orm.exc import NoResultFound
 
 from app.dao import services_dao, templates_dao
 from app.dao.service_sms_sender_dao import dao_get_service_sms_sender_by_id
@@ -228,6 +229,7 @@ def check_service_email_reply_to_id(
             raise BadRequestError(message=message)
 
 
+@cached(cache=TTLCache(maxsize=1024, ttl=600))
 def check_service_sms_sender_id(
     service_id,
     sms_sender_id,

app/template/rest.py

@@ -18,6 +18,7 @@
 from sqlalchemy.orm.exc import NoResultFound
 from notifications_utils.template import HTMLEmailTemplate
 
+from app import HTTP_TIMEOUT
 from app.authentication.auth import requires_admin_auth_or_user_in_service, requires_user_in_service_or_admin
 from app.communication_item import validate_communication_items
 from app.dao.fact_notification_status_dao import fetch_template_usage_for_service_with_given_template
@@ -453,14 +454,14 @@ def _get_png_preview_or_overlaid_pdf(
             url,
             json=data,
             headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])},
-            timeout=(3.05, 1),
+            timeout=HTTP_TIMEOUT,
         )
     else:
         resp = requests_post(
             url,
             data=data,
             headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])},
-            timeout=(3.05, 1),
+            timeout=HTTP_TIMEOUT,
         )
 
     if resp.status_code != 200:

app/user/rest.py

@@ -11,7 +11,7 @@
 from sqlalchemy.exc import IntegrityError
 from urllib.parse import urlencode
 
-from app import db
+from app import db, HTTP_TIMEOUT
 from app.config import QueueNames, Config
 from app.dao.fido2_key_dao import (
     save_fido2_key,
@@ -438,7 +438,10 @@ def send_support_email(user_id):
     }
 
     response = requests.post(
-        '{}/api/v2/tickets'.format(API_URL), json=ticket, auth=requests.HTTPBasicAuth(API_KEY, 'x'), timeout=(3.05, 1)
+        '{}/api/v2/tickets'.format(API_URL),
+        json=ticket,
+        auth=requests.HTTPBasicAuth(API_KEY, 'x'),
+        timeout=HTTP_TIMEOUT,
     )
 
     if response.status_code != 201:

app/va/mpi/mpi.py

@@ -5,6 +5,7 @@
 from time import monotonic
 from http.client import responses
 from functools import reduce
+
 from app.va.identifier import (
     IdentifierType,
     transform_to_fhir_format,
@@ -82,6 +83,9 @@ def init_app(
         ssl_key_path,
         statsd_client,
     ):
+        from app import HTTP_TIMEOUT  # Circular import
+
+        self.timeout = HTTP_TIMEOUT
         self.logger = logger
         self.base_url = url
         self.ssl_cert_path = ssl_cert_path
@@ -132,7 +136,7 @@ def _make_request(
                     f'{self.base_url}/psim_webservice/fhir/Patient/{fhir_identifier}',
                     params={'-sender': self.SYSTEM_IDENTIFIER},
                     cert=(self.ssl_cert_path, self.ssl_key_path),
-                    timeout=(3.05, 5),
+                    timeout=self.timeout,
                 )
 
                 response.raise_for_status()

app/va/va_onsite/va_onsite_client.py

@@ -19,6 +19,9 @@ def init_app(
         :param url: the url to send the information to in a string format
         :param va_onsite_secret: the secret key in string format used to validate the connection
         """
+        from app import HTTP_TIMEOUT  # Circular import
+
+        self.timeout = HTTP_TIMEOUT
         self.logger = logger
         self.url_base = url
         self.va_onsite_secret = va_onsite_secret
@@ -40,7 +43,7 @@ def post_onsite_notification(
                 url=f'{ self.url_base }/v0/onsite_notifications',
                 data=json.dumps(data),
                 headers=self._build_header(),
-                timeout=(3.05, 1),
+                timeout=self.timeout,
             )
         except Exception as e:
             self.logger.exception(e)

app/va/va_profile/va_profile_client.py

@@ -86,6 +86,9 @@ def init_app(
         va_profile_token,
         statsd_client,
     ):
+        from app import HTTP_TIMEOUT  # Circular import
+
+        self.timeout = HTTP_TIMEOUT
         self.logger: Logger = logger
         self.va_profile_url = va_profile_url
         self.ssl_cert_path = ssl_cert_path
@@ -109,7 +112,7 @@ def get_profile(self, va_profile_id: RecipientIdentifier) -> Profile:
         data = {'bios': [{'bioPath': 'contactInformation'}, {'bioPath': 'communicationPermissions'}]}
 
         try:
-            response = requests.post(url, json=data, cert=(self.ssl_cert_path, self.ssl_key_path), timeout=(3.05, 1))
+            response = requests.post(url, json=data, cert=(self.ssl_cert_path, self.ssl_key_path), timeout=self.timeout)
             response.raise_for_status()
         except (requests.HTTPError, requests.RequestException, requests.Timeout) as e:
             self._handle_exceptions(va_profile_id.id_value, e)
@@ -339,10 +342,13 @@ def _handle_exceptions(self, va_profile_id_value: str, error: Exception):
 
         elif isinstance(error, requests.RequestException):
             self.statsd_client.incr('clients.va-profile.error.request_exception')
-            failure_message = 'VA Profile returned RequestException while querying for VA Profile ID'
+            failure_message = f'VA Profile returned {error.__class__.__name__} while querying for VA Profile ID'
 
             if isinstance(error, requests.Timeout):
-                failure_message = f'VA Profile request timed out for VA Profile ID {va_profile_id_value}.'
+                failure_message = (
+                    f'VA Profile request timed out with {error.__class__.__name__} '
+                    f'for VA Profile ID {va_profile_id_value}.'
+                )
 
             exception = VAProfileRetryableException(failure_message)
             exception.failure_reason = failure_message
@@ -370,7 +376,7 @@ def send_va_profile_email_status(self, notification_data: dict) -> None:
         # make POST request to VA Profile endpoint for notification statuses
         # raise errors if they occur, they will be handled by the calling function
         try:
-            response = requests.post(url, json=notification_data, headers=headers, timeout=(3.05, 1))
+            response = requests.post(url, json=notification_data, headers=headers, timeout=self.timeout)
         except requests.Timeout:
             self.logger.exception(
                 'Request timeout attempting to send email status to VA Profile for notification %s | retrying...',