From ed30a85bc8ba660652fae862f8b9dbf463356002 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 3 Dec 2024 12:44:57 -0800 Subject: [PATCH 1/6] change create notification to an upsert --- app/dao/notifications_dao.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index cbde45d30..4c40a7e81 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -71,7 +71,9 @@ def dao_create_notification(notification): # notify-api-742 remove phone numbers from db notification.to = "1" notification.normalised_to = "1" - db.session.add(notification) + + # notify-api-1454 change to an upsert + db.session.merge(notification) def country_records_delivery(phone_prefix): From b4093e8152a359e08b7960fd8a87f614928ad361 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 3 Dec 2024 12:55:52 -0800 Subject: [PATCH 2/6] change create notification to an upsert --- app/dao/notifications_dao.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4c40a7e81..d231ece42 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -73,7 +73,10 @@ def dao_create_notification(notification): notification.normalised_to = "1" # notify-api-1454 change to an upsert - db.session.merge(notification) + stmt = select(Notification).where(Notification.id == notification.id) + result = db.session.execute(stmt).scalar() + if result is None: + db.session.add(notification) def country_records_delivery(phone_prefix): From 14e4d761fc4daf769f0cd346d0682ba05131f634 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 3 Dec 2024 13:50:42 -0800 Subject: [PATCH 3/6] try to fix tests --- app/celery/tasks.py | 16 +++++++++++----- app/dao/notifications_dao.py | 2 +- app/notifications/process_notifications.py | 5 +++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 0794ad4da..8cece7aa4 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -20,7 +20,10 @@ from app.dao.templates_dao import dao_get_template_by_id from app.enums import JobStatus, KeyType, NotificationType from app.errors import TotalRequestsError -from app.notifications.process_notifications import persist_notification +from app.notifications.process_notifications import ( + get_notification, + persist_notification, +) from app.notifications.validators import check_service_over_total_message_limit from app.serialised_models import SerialisedService, SerialisedTemplate from app.service.utils import service_allowed_to_send_to @@ -329,6 +332,8 @@ def save_api_email_or_sms(self, encrypted_notification): if notification["notification_type"] == NotificationType.EMAIL else provider_tasks.deliver_sms ) + + original_notification = get_notification(notification["id"]) try: persist_notification( notification_id=notification["id"], @@ -347,10 +352,11 @@ def save_api_email_or_sms(self, encrypted_notification): document_download_count=notification["document_download_count"], ) # Only get here if save to the db was successful (i.e. first time) - provider_task.apply_async([notification["id"]], queue=q) - current_app.logger.debug( - f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue." - ) + if original_notification is None: + provider_task.apply_async([notification["id"]], queue=q) + current_app.logger.debug( + f"{notification['id']} has been persisted and sent to delivery queue." + ) except IntegrityError: current_app.logger.warning( diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d231ece42..8371aaa85 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -72,7 +72,7 @@ def dao_create_notification(notification): notification.to = "1" notification.normalised_to = "1" - # notify-api-1454 change to an upsert + # notify-api-1454 insert only if it doesn't exist stmt = select(Notification).where(Notification.id == notification.id) result = db.session.execute(stmt).scalar() if result is None: diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 4f5d8d06c..8568b8894 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -8,6 +8,7 @@ from app.dao.notifications_dao import ( dao_create_notification, dao_delete_notifications_by_id, + get_notification_by_id, ) from app.enums import KeyType, NotificationStatus, NotificationType from app.errors import BadRequestError @@ -53,6 +54,10 @@ def check_placeholders(template_object): raise BadRequestError(fields=[{"template": message}], message=message) +def get_notification(notification_id): + return get_notification_by_id(notification_id) + + def persist_notification( *, template_id, From 9b1eb70bbebff28a07f66d15a9a3951d709b5f59 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 3 Dec 2024 14:05:25 -0800 Subject: [PATCH 4/6] try to fix tests --- app/celery/tasks.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 8cece7aa4..7526e6619 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -274,7 +274,7 @@ def save_email( "Email {} failed as restricted service".format(notification_id) ) return - + original_notification = get_notification(notification_id) try: saved_notification = persist_notification( template_id=notification["template"], @@ -291,10 +291,11 @@ def save_email( notification_id=notification_id, reply_to_text=reply_to_text, ) - - provider_tasks.deliver_email.apply_async( - [str(saved_notification.id)], queue=QueueNames.SEND_EMAIL - ) + # we only want to send once + if original_notification is None: + provider_tasks.deliver_email.apply_async( + [str(saved_notification.id)], queue=QueueNames.SEND_EMAIL + ) current_app.logger.debug( "Email {} created at {}".format( From 55f538b10f71bcefb1a8d5c71c1a3e68152499d0 Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Tue, 3 Dec 2024 17:28:36 -0500 Subject: [PATCH 5/6] Update Restage workflow to use latest cg-cli-tools This changeset updates our restage workflow and GitHub action to use the latest version of the cg-cli-tools to help prevent future issues with performing restage actions for our apps. Signed-off-by: Carlo Costino --- .github/workflows/restage-apps.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/restage-apps.yml b/.github/workflows/restage-apps.yml index abdadcfe0..23c78f8cf 100644 --- a/.github/workflows/restage-apps.yml +++ b/.github/workflows/restage-apps.yml @@ -19,18 +19,18 @@ jobs: app: ["api", "admin"] steps: - name: Restage ${{matrix.app}} - uses: 18f/cg-deploy-action@main + uses: cloud-gov/cg-cli-tools@main with: cf_username: ${{ secrets.CLOUDGOV_USERNAME }} cf_password: ${{ secrets.CLOUDGOV_PASSWORD }} cf_org: gsa-tts-benefits-studio cf_space: notify-${{ inputs.environment }} - full_command: "cf restage --strategy rolling notify-${{matrix.app}}-${{inputs.environment}}" + command: "cf restage --strategy rolling notify-${{matrix.app}}-${{inputs.environment}}" - name: Restage ${{matrix.app}} egress - uses: 18f/cg-deploy-action@main + uses: cloud-gov/cg-cli-tools@main with: cf_username: ${{ secrets.CLOUDGOV_USERNAME }} cf_password: ${{ secrets.CLOUDGOV_PASSWORD }} cf_org: gsa-tts-benefits-studio cf_space: notify-${{ inputs.environment }}-egress - full_command: "cf restage --strategy rolling egress-proxy-notify-${{matrix.app}}-${{inputs.environment}}" + command: "cf restage --strategy rolling egress-proxy-notify-${{matrix.app}}-${{inputs.environment}}" From 2036e575e0018585f9335794d0cde92cc62075f3 Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Tue, 3 Dec 2024 20:57:25 -0500 Subject: [PATCH 6/6] Bump Redis plan to 5node-large This changeset boosts our production Redis configuration to the 5node-large plan to give us a bit more wiggle room for heavy production workloads. This should still keep us in the threshold of the second block of 10 nodes we are already signed up for as we currently have 12 nodes going across all environments at the moment. Signed-off-by: Carlo Costino --- terraform/production/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/production/main.tf b/terraform/production/main.tf index 9543fdd86..d8a46693b 100644 --- a/terraform/production/main.tf +++ b/terraform/production/main.tf @@ -27,7 +27,7 @@ module "redis-v70" { cf_org_name = local.cf_org_name cf_space_name = local.cf_space_name name = "${local.app_name}-redis-v70-${local.env}" - redis_plan_name = "redis-3node-large" + redis_plan_name = "redis-5node-large" json_params = jsonencode( { "engineVersion" : "7.0",