Merge remote-tracking branch 'origin/main' into monkeytype-auto-annotate

.vscode/launch.json

@@ -43,7 +43,7 @@
                 "-l",
                 "DEBUG",
                 "-Q",
-                "database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts",
+                "database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks-retry,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts",
             ]
         },
         {

Makefile

@@ -48,19 +48,19 @@ smoke-test:
 
 .PHONY: run
 run: ## Run the web app
-	flask run -p 6011 --host=0.0.0.0
+	poetry run flask run -p 6011 --host=0.0.0.0
 
 .PHONY: run-celery-local
 run-celery-local: ## Run the celery workers with all the queues
-	./scripts/run_celery_local.sh
+	poetry run ./scripts/run_celery_local.sh
 
 .PHONY: run-celery-local-filtered
 run-celery-local-filtered: ## Run the celery workers with all queues but filter out common scheduled tasks
-	./scripts/run_celery_local.sh 2>&1 >/dev/null | grep -iEv 'beat|in-flight-to-inbox|run-scheduled-jobs|check-job-status'
+	poetry run ./scripts/run_celery_local.sh 2>&1 >/dev/null | grep -iEv 'beat|in-flight-to-inbox|run-scheduled-jobs|check-job-status'
 
 .PHONY: run-celery-purge
 run-celery-purge: ## Purge the celery queues
-	./scripts/run_celery_purge.sh
+	poetry run ./scripts/run_celery_purge.sh
 
 .PHONY: run-db
 run-db: ## psql to access dev database

README.md

@@ -17,78 +17,7 @@ Contains:
 For any issues during the following instructions, make sure to review the
 **Frequent problems** section toward the end of the document.
 
-### Local installation instruction
-
-#### On OS X:
-
-1. Install PyEnv with Homebrew. This will preserve your sanity.
-
-`brew install pyenv`
-
-2. Install Python 3.10.8 or whatever is the latest
-
-`pyenv install 3.10.8`
-
-3. If you expect no conflicts, set `3.10.8` as you default
-
-`pyenv global 3.10.8`
-
-4. Ensure it installed by running
-
-`python --version`
-
-if it did not, take a look here: https://github.com/pyenv/pyenv/issues/660
-
-5. Install `poetry`:
-
-`pip install poetry==1.3.2`
-
-6. Restart your terminal and make your virtual environtment:
-
-`poetry env use $(which python)`
-
-8. Verify that the environment was created and activated by poetry
-
-`poetry env list`
-
-9. Install [Postgres.app](http://postgresapp.com/).
-
-10. Create the database for the application
-
-`createdb --user=postgres notification_api`
-
-11. Install the required environment variables via our LastPast Vault
-
-Within the team's *LastPass Vault*, you should find corresponding folders for this
-project containing the `.env` content that you should copy in your project root folder. This
-will grant the application necessary access to our internal infrastructure.
-
-If you don't have access to our *LastPass Vault* (as you evaluate our notification
-platform for example), you will find a sane set of defaults exists in the `.env.example`
-file. Copy that file to `.env` and customize it to your needs.
-
-12. Install all dependencies
-
-`poetry install`
-
-1.  Generate the version file ?!?
-
-`make generate-version-file`
-
-14. Run all DB migrations
-
-`flask db upgrade`
-
-15. Run the service
-
-`make run`
-
-15a. To test
-
-`poetry install --with test`
-
-`make test`
-
+### Local installation instruction (Use Dev Containers)
 #### In a [VS Code devcontainer](https://code.visualstudio.com/docs/remote/containers-tutorial)
 
 1. Install VS Code

app/celery/service_callback_tasks.py

@@ -65,31 +65,23 @@ def _send_data_to_service_callback_api(self, data, service_callback_url, token,
             data=json.dumps(data),
             headers={
                 "Content-Type": "application/json",
-                "Authorization": "Bearer {}".format(token),
+                "Authorization": f"Bearer {token}",
             },
-            timeout=60,
+            timeout=5,
         )
         current_app.logger.info(
-            "{} sending {} to {}, response {}".format(
-                function_name,
-                notification_id,
-                service_callback_url,
-                response.status_code,
-            )
+            f"{function_name} sending {notification_id} to {service_callback_url}, response {response.status_code}"
         )
         response.raise_for_status()
     except RequestException as e:
         current_app.logger.warning(
-            "{} request failed for notification_id: {} and url: {}. exc: {}".format(
-                function_name, notification_id, service_callback_url, e
-            )
+            f"{function_name} request failed for notification_id: {notification_id} and url: {service_callback_url}. exc: {e}"
         )
-        if not isinstance(e, HTTPError) or e.response.status_code >= 500:
+        # Retry if the response status code is server-side or 429 (too many requests).
+        if not isinstance(e, HTTPError) or e.response.status_code >= 500 or e.response.status_code == 429:
             try:
-                self.retry(queue=QueueNames.RETRY)
+                self.retry(queue=QueueNames.CALLBACKS_RETRY)
             except self.MaxRetriesExceededError:
                 current_app.logger.warning(
-                    "Retry: {} has retried the max num of times for callback url {} and notification_id: {}".format(
-                        function_name, service_callback_url, notification_id
-                    )
+                    "Retry: {function_name} has retried the max num of times for callback url {service_callback_url} and notification_id: {notification_id}"
                 )

app/config.py

@@ -107,6 +107,7 @@ class QueueNames(object):
     PROCESS_FTP = "process-ftp-tasks"
     CREATE_LETTERS_PDF = "create-letters-pdf-tasks"
     CALLBACKS = "service-callbacks"
+    CALLBACKS_RETRY = "service-callbacks-retry"
 
     # Queue for letters, unused by CDS at this time as we don't use these.
     LETTERS = "letter-tasks"
@@ -156,6 +157,7 @@ def all_queues():
             QueueNames.REPORTING,
             QueueNames.JOBS,
             QueueNames.RETRY,
+            QueueNames.CALLBACKS_RETRY,
             QueueNames.NOTIFY,
             # QueueNames.CREATE_LETTERS_PDF,
             QueueNames.CALLBACKS,

scripts/run_celery.ps1

@@ -1,3 +1,3 @@
 $ENV:FORKED_BY_MULTIPROCESSING=1
 
-celery --app run_celery worker --pidfile="$env:TEMP\celery.pid" --pool=solo --loglevel=DEBUG --concurrency=1 -Q "database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,delivery-receipts"
+celery --app run_celery worker --pidfile="$env:TEMP\celery.pid" --pool=solo --loglevel=DEBUG --concurrency=1 -Q "database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,service-callbacks-retry,delivery-receipts"

scripts/run_celery.sh

@@ -6,4 +6,4 @@ set -e
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,service-callbacks-retry,delivery-receipts

scripts/run_celery_core_tasks.sh

@@ -7,4 +7,4 @@ set -e
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,service-callbacks-retry,delivery-receipts

scripts/run_celery_local.sh

@@ -7,4 +7,4 @@ set -e
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --beat --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --beat --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,service-callbacks-retry,delivery-receipts

scripts/run_celery_no_sms_sending.sh

@@ -28,4 +28,4 @@ fi
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,service-callbacks-retry,delivery-receipts

tests/app/celery/test_service_callback_tasks.py

@@ -90,8 +90,9 @@ def test_send_complaint_to_service_posts_https_request_to_service_with_signed_da
 
 
 @pytest.mark.parametrize("notification_type", ["email", "letter", "sms"])
-def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_signed_data(
-    notify_db_session, mocker, notification_type
+@pytest.mark.parametrize("status_code", [429, 500, 503])
+def test__send_data_to_service_callback_api_retries_if_request_returns_error_code_with_signed_data(
+    notify_db_session, mocker, notification_type, status_code
 ):
     callback_api, template = _set_up_test_data(notification_type, "delivery_status")
     datestr = datetime(2017, 6, 20)
@@ -107,11 +108,11 @@ def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_
     signed_data = _set_up_data_for_status_update(callback_api, notification)
     mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.retry")
     with requests_mock.Mocker() as request_mock:
-        request_mock.post(callback_api.url, json={}, status_code=500)
+        request_mock.post(callback_api.url, json={}, status_code=status_code)
         send_delivery_status_to_service(notification.id, signed_status_update=signed_data)
 
     assert mocked.call_count == 1
-    assert mocked.call_args[1]["queue"] == "retry-tasks"
+    assert mocked.call_args[1]["queue"] == "service-callbacks-retry"
 
 
 @pytest.mark.parametrize("notification_type", ["email", "letter", "sms"])

tests/app/test_config.py

@@ -27,7 +27,7 @@ def reload_config():
 def test_queue_names_all_queues_correct():
     # Need to ensure that all_queues() only returns queue names used in API
     queues = QueueNames.all_queues()
-    assert len(queues) == 21
+    assert len(queues) == 22
     assert set(
         [
             QueueNames.PRIORITY,
@@ -48,6 +48,7 @@ def test_queue_names_all_queues_correct():
             QueueNames.REPORTING,
             QueueNames.JOBS,
             QueueNames.RETRY,
+            QueueNames.CALLBACKS_RETRY,
             QueueNames.NOTIFY,
             # QueueNames.CREATE_LETTERS_PDF,
             QueueNames.CALLBACKS,