From fbe733bc7eeca1ccd454fe4752c9d54f7bb9d1c6 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 9 Jun 2025 07:12:05 -0700 Subject: [PATCH 1/5] check which services have csvs --- app/aws/s3.py | 30 ++++++++++++++++++++++++++++++ app/celery/tasks.py | 11 ++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index d98529d8a..2a51038fc 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -11,6 +11,8 @@ from flask import current_app from app import job_cache, job_cache_lock from app.clients import AWS_CLIENT_CONFIG + +# from app.service.rest import get_service_by_id from notifications_utils import aware_utcnow FILE_LOCATION_STRUCTURE = "service-{}-notify/{}.csv" @@ -162,6 +164,34 @@ def cleanup_old_s3_objects(): current_app.logger.exception( "#delete-old-s3-objects An error occurred while cleaning up old s3 objects", ) + try: + response = s3_client.list_objects_v2(Bucket=bucket_name) + + service_ids = set() + while True: + for obj in response.get("Contents", []): + # Get the service id out of the upload key + key = obj["Key"] + object_arr = key.split("/") + service_id = object_arr[0] + service_id = service_id.replace("-service-notify", "") + service_ids.add(service_id) + if "NextContinuationToken" in response: + response = s3_client.list_objects_v2( + Bucket=bucket_name, + ContinuationToken=response["NextContinuationToken"], + ) + else: + break + retained_services = [] + for service_id in service_ids: + retained_services.append(service_id) + + return service_ids + except Exception: + current_app.logger.exception( + "#delete-old-s3-objects An error occurred while cleaning up old s3 objects", + ) def get_job_id_from_s3_object_key(key): diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 4a5311c50..61f7b5a20 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -19,6 +19,7 @@ from app.dao.notifications_dao import ( from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id +from app.dao.services_dao import dao_fetch_service_by_id from app.dao.templates_dao import dao_get_template_by_id from app.enums import JobStatus, KeyType, NotificationType from app.errors import TotalRequestsError @@ -496,7 +497,15 @@ def clean_job_cache(): @notify_celery.task(name="delete-old-s3-objects") def delete_old_s3_objects(): - s3.cleanup_old_s3_objects() + + existing_service_ids = s3.cleanup_old_s3_objects() + service_names = [] + for service_id in existing_service_ids: + service = dao_fetch_service_by_id(service_id) + service_names.append(service.name) + current_app.logger.info( + f"#delete-old-s3-objects Services with retained csvs: {service_names}" + ) @notify_celery.task(name="process-incomplete-jobs") From a1bf2918dc5f34832f7ea2170c81875cff1bd7fd Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Tue, 10 Jun 2025 14:35:48 -0400 Subject: [PATCH 2/5] Add new ADR 0014: Localize the notifications-python-client library This changeset formalizes the new ADR (Architectural Decision Record) that proposed localizing the notifications-python-client into the API and Admin applications. Signed-off-by: Carlo Costino --- ...dr-localize-notifications-python-client.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/adrs/0014-adr-localize-notifications-python-client.md diff --git a/docs/adrs/0014-adr-localize-notifications-python-client.md b/docs/adrs/0014-adr-localize-notifications-python-client.md new file mode 100644 index 000000000..627fb86aa --- /dev/null +++ b/docs/adrs/0014-adr-localize-notifications-python-client.md @@ -0,0 +1,51 @@ +# Bring in the notifications-python-client directly to the API and Admin apps + +Status: Accepted +Date: 10 June 2025 + +### Context + +We still pull in the `notifications-python-client` as a third party library, and it is what underpins all of the requests made throughout the API and admin applications. Our apps have diverged enough from the original UK Notify applications that we ought to consider making our own copy of this library for our apps to make sure the client continues to meet our use cases and we aren't suddenly caught in a bind with an update that won't work for us. + +Furthermore, we need to make some adjustments to this library in order to continue updating Python (e.g. Python 3.13) due to some incompatibilities, such as how SSL certificates are validated. + +### Decision + +We're going to pull in the `notifications-python-client` library (source code found here: https://github.com/alphagov/notifications-python-client to both the API and Admin apps just like we did with the `notifications-utils` library/repo. This will involve doing the following: + +* Making a local copy of the `https://github.com/alphagov/notifications-python-client` code base (the code for the library itself - likely just what's in https://github.com/alphagov/notifications-python-client/tree/main/notifications_python_client but we need to double check) within a `notifications_python_client` folder at the root of the project directory. +* Incorporating any of the dependencies required for the library into our own directly (in the `pyproject.toml` file) that aren't already accounted for. +* Making sure all namespaces (e.g., `import` statements) within our app for references to the library continue to work still. +* Make sure tests are included/excluded as appropriate (similar to what we did with `notifications_utils`). + +### Consequences + +We anticipate the impacts to our project and team to be the following: + +* No longer gaining any updates the library directly that are published to PyPI. +* Slight increased burden in codebase maintenance. +* A bit of extra work to incorporate the library in fully and completely to the API and Admin directly. +* No longer sharing code between the API and Admin; any changes made to the `notifications-python-client` in one app will need to be mirrored in the other. + +However, we also anticipate these benefits in doing this work: + +* Gaining full control of future changes to the `notifications-python-client` code. +* Ability to reduce the `notifications-python-client`'s footprint for our own needs and use cases. +* Ability to make changes and updates necessary to keep the other parts of the application up-to-date (e.g., Python updates) +* Removing reliance on a third-party dependency that we have no control over. + +### Author + +@ccostino + +### Stakeholders + +@ccostino + +### Next Steps + +Next steps once this draft ADR is posted: + +* Team reviews the ADR and makes adjustments as necessary +* Team agrees on the approach of the ADR and finalizes it for acceptance (creates a proper ADR file for it) +* Issue(s) get created to perform the work in the API and the Admin From fe1f333d4cbc73e89ca384435c80ed7dad8a0a5b Mon Sep 17 00:00:00 2001 From: Kenneth Kehl Date: Wed, 11 Jun 2025 07:27:51 -0700 Subject: [PATCH 3/5] Update app/aws/s3.py Co-authored-by: ccostino --- app/aws/s3.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index 2a51038fc..95629c5f5 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -188,9 +188,9 @@ def cleanup_old_s3_objects(): retained_services.append(service_id) return service_ids - except Exception: + except Exception as error: current_app.logger.exception( - "#delete-old-s3-objects An error occurred while cleaning up old s3 objects", + f"#delete-old-s3-objects An error occurred while cleaning up old s3 objects: str(error)" ) From 0044a1580a256fb669a716d01d3b9415f208548d Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 11 Jun 2025 07:39:32 -0700 Subject: [PATCH 4/5] ugh --- app/aws/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index 95629c5f5..c2680624d 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -190,7 +190,7 @@ def cleanup_old_s3_objects(): return service_ids except Exception as error: current_app.logger.exception( - f"#delete-old-s3-objects An error occurred while cleaning up old s3 objects: str(error)" + f"#delete-old-s3-objects An error occurred while cleaning up old s3 objects: {str(error)}" ) From eda0612561c3cd205779fc3497b8fed29a34c684 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 11 Jun 2025 21:36:15 +0000 Subject: [PATCH 5/5] Bump faker from 37.3.0 to 37.4.0 Bumps [faker](https://github.com/joke2k/faker) from 37.3.0 to 37.4.0. - [Release notes](https://github.com/joke2k/faker/releases) - [Changelog](https://github.com/joke2k/faker/blob/master/CHANGELOG.md) - [Commits](https://github.com/joke2k/faker/compare/v37.3.0...v37.4.0) --- updated-dependencies: - dependency-name: faker dependency-version: 37.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index c3dbf9146..d9271533a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1407,14 +1407,14 @@ tests = ["coverage", "coveralls", "dill", "mock", "nose"] [[package]] name = "faker" -version = "37.3.0" +version = "37.4.0" description = "Faker is a Python package that generates fake data for you." optional = false python-versions = ">=3.9" groups = ["main"] files = [ - {file = "faker-37.3.0-py3-none-any.whl", hash = "sha256:48c94daa16a432f2d2bc803c7ff602509699fca228d13e97e379cd860a7e216e"}, - {file = "faker-37.3.0.tar.gz", hash = "sha256:77b79e7a2228d57175133af0bbcdd26dc623df81db390ee52f5104d46c010f2f"}, + {file = "faker-37.4.0-py3-none-any.whl", hash = "sha256:cb81c09ebe06c32a10971d1bbdb264bb0e22b59af59548f011ac4809556ce533"}, + {file = "faker-37.4.0.tar.gz", hash = "sha256:7f69d579588c23d5ce671f3fa872654ede0e67047820255f43a4aa1925b89780"}, ] [package.dependencies] @@ -5587,4 +5587,4 @@ cffi = ["cffi (>=1.11)"] [metadata] lock-version = "2.1" python-versions = "^3.12.2" -content-hash = "c965d17cdc92181a8b6cce13218edaf8fae0444a1f0815a7132f305a1b1383a9" +content-hash = "c33fc9597a1f32253ff108478bc201374dd834effbae4b8fbbccc7a5140b3a18" diff --git a/pyproject.toml b/pyproject.toml index 45d2fb4cb..197d6a5e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,7 @@ pyjwt = "==2.10.1" python-dotenv = "==1.1.0" sqlalchemy = "==2.0.41" werkzeug = "^3.0.6" -faker = "^37.3.0" +faker = "^37.4.0" async-timeout = "^5.0.1" bleach = "^6.1.0" geojson = "^3.2.0"